From e65bd303394b4aef64529a460ccdafef800c9199 Mon Sep 17 00:00:00 2001 From: Kiril Videlov Date: Sat, 4 Jul 2026 10:06:08 +0200 Subject: [PATCH 1/3] Add stateless conflict inspection and per-hunk resolution Two new APIs on the conflict primitives: but_api::resolve::commit_conflicts returns a conflicted commit's conflicts as per-file ours/base/theirs hunks, and resolve_commit_conflict_hunks applies side picks or custom content to any subset of them. Partial resolutions narrow the conflict: each resolved hunk is written into all three side trees and the re-merge conflicts only at what remains, so the commit stays conflicted with fewer conflicts and work can proceed incrementally. Full coverage produces a normal commit via the same path, and the AI resolver now runs on this core. Exposed as `but resolve conflicts` and `but resolve apply [:]` (side flags or stdin/file content, defaulting to the oldest conflicted commit), as a read-only conflicts modal on right-click in the desktop app. The agent skill now steers coding agents to this stateless flow, with edit mode reserved for when the commit must be checked out. --- .../commit/CommitContextMenu.svelte | 16 + .../commit/ConflictHunksModal.svelte | 104 +++ apps/desktop/src/lib/stacks/stackEndpoints.ts | 11 + .../src/lib/stacks/stackService.svelte.ts | 4 + crates/but-api/src/resolve/apply.rs | 618 ++++++++++++------ crates/but-api/src/resolve/context.rs | 23 +- crates/but-api/src/resolve/mod.rs | 385 +++++++++-- crates/but-api/src/resolve/prompt.rs | 4 +- crates/but-api/tests/api/main.rs | 1 + crates/but-api/tests/api/resolve_ai.rs | 40 +- crates/but-api/tests/api/resolve_hunks.rs | 209 ++++++ .../scenario/resolve-ai-conflicted-commit.sh | 11 +- crates/but/skill/SKILL.md | 22 +- crates/but/src/args/resolve.rs | 38 ++ crates/but/src/command/legacy/oplog.rs | 2 + crates/but/src/command/legacy/resolve.rs | 350 ++++++++++ crates/gitbutler-oplog/src/entry.rs | 5 + crates/gitbutler-tauri/src/main.rs | 2 + .../but-sdk/src/generated/graph/index.d.ts | 83 ++- .../but-sdk/src/generated/linear/index.d.ts | 83 ++- packages/ui/src/lib/utils/testIds.ts | 1 + 21 files changed, 1745 insertions(+), 267 deletions(-) create mode 100644 apps/desktop/src/components/commit/ConflictHunksModal.svelte create mode 100644 crates/but-api/tests/api/resolve_hunks.rs diff --git a/apps/desktop/src/components/commit/CommitContextMenu.svelte b/apps/desktop/src/components/commit/CommitContextMenu.svelte index 8bd85af3bcf..64186501a0b 100644 --- a/apps/desktop/src/components/commit/CommitContextMenu.svelte +++ b/apps/desktop/src/components/commit/CommitContextMenu.svelte @@ -53,6 +53,7 @@ + + + {#if conflictsQuery} + + {#snippet children(conflicts)} +
+ {#each conflicts.files as file (file.path)} +
+

{file.path}

+ {#each file.hunks as hunk, index} +
+
+ Conflict {index + 1} of {file.hunks.length} +
+
ours — the new base
+ 
{hunk.ours}
+ {#if hunk.base !== null} +
base — common ancestor
+ 
{hunk.base}
+ {/if} +
theirs — this commit
+ 
{hunk.theirs}
+
+ {/each} +
+ {/each} +
+ {/snippet} + + {/if} + + {#snippet controls(close)} + + {/snippet} + + + diff --git a/apps/desktop/src/lib/stacks/stackEndpoints.ts b/apps/desktop/src/lib/stacks/stackEndpoints.ts index 14c6fb238e3..8e13ce68976 100644 --- a/apps/desktop/src/lib/stacks/stackEndpoints.ts +++ b/apps/desktop/src/lib/stacks/stackEndpoints.ts @@ -27,6 +27,7 @@ import type { AbsorptionTarget, AiResolutionResult, BranchLandResult, + CommitConflicts, CommitAbsorption, BranchDetails, BranchReference, @@ -455,6 +456,16 @@ export function buildStackEndpoints(build: BackendEndpointBuilder) { ...(stackId ? [invalidatesItem(ReduxTag.StackDetails, stackId)] : []), ], }), + commitConflicts: build.query({ + // Conflicts are derived from the commit's own trees, so results are + // immutable per commit id. + keepUnusedDataFor: 60, + extraOptions: { command: "commit_conflicts" }, + query: (args) => args, + providesTags: (_result, _error, { commitId }) => [ + ...providesItem(ReduxTag.CommitChanges, commitId), + ], + }), resolveCommitConflictsAi: build.mutation< AiResolutionResult, { projectId: string; stackId?: string; commitId: string } diff --git a/apps/desktop/src/lib/stacks/stackService.svelte.ts b/apps/desktop/src/lib/stacks/stackService.svelte.ts index edde4147d95..094fe7b03c8 100644 --- a/apps/desktop/src/lib/stacks/stackService.svelte.ts +++ b/apps/desktop/src/lib/stacks/stackService.svelte.ts @@ -554,6 +554,10 @@ export class StackService { return this.backendApi.endpoints.resolveCommitConflictsAi.useMutation(); } + commitConflicts(projectId: string, commitId: string) { + return this.backendApi.endpoints.commitConflicts.useQuery({ projectId, commitId }); + } + get newBranch() { return this.backendApi.endpoints.newBranch.useMutation(); } diff --git a/crates/but-api/src/resolve/apply.rs b/crates/but-api/src/resolve/apply.rs index 411d83adb14..aeed0976495 100644 --- a/crates/but-api/src/resolve/apply.rs +++ b/crates/but-api/src/resolve/apply.rs @@ -1,122 +1,162 @@ -//! Validate a model response against the request, splice the resolved hunks -//! into the merged blobs, and rewrite the conflicted commit into a normal one. +//! Turn per-hunk resolutions into a rewritten commit. +//! +//! Resolutions are applied by *narrowing the conflict*: each resolved hunk's +//! content is written into all three side blobs (ours/base/theirs), while +//! unresolved hunks keep each side's original lines. Re-merging the synthesized +//! trees then conflicts exactly at the unresolved hunks — so a partial +//! resolution yields a conflicted commit with fewer conflicts, and a full +//! resolution falls out of the very same path as a clean merge. + +use std::collections::BTreeMap; use anyhow::{Context as _, bail}; use bstr::ByteSlice; use but_core::DryRun; use but_core::commit::Headers; +use but_core::commit::tree_expression::TreeExpression; use but_core::sync::RepoExclusive; use but_rebase::commit::DateMode; use but_rebase::graph_rebase::{Editor, LookupStep as _, Step}; use super::context::{FileConflict, ResolutionRequest, is_marker_shaped, scan_conflict_blocks}; -use super::prompt::ResolutionResponse; +use super::{HunkResolution, RemainingConflicts, ResolutionSpec}; use crate::WorkspaceState; -/// A validated, spliced resolution, aligned index-for-index with the -/// request's files. -#[derive(Debug)] -pub(crate) struct ValidatedFile { - /// The full file content with every conflict block replaced by its resolution. - pub resolved_text: String, - /// The per-hunk replacement contents, in file order. - pub hunks: Vec, - /// The model's per-file reasoning. - pub reasoning: String, +/// What applying resolutions produced. +pub(crate) struct AppliedResolution { + /// The rewritten commit's final id after the rebase. + pub new_commit: gix::ObjectId, + /// The conflicts that remain per file; empty when fully resolved. + pub remaining: Vec, + /// Workspace state after the apply. + pub workspace: WorkspaceState, } -/// Check `response` against `request` and splice the resolutions into the -/// merged blobs. Any mismatch fails validation so the caller can retry the -/// model once before giving up. Nothing is written to the repository here. -pub(crate) fn validate( - request: &ResolutionRequest, - response: &ResolutionResponse, -) -> anyhow::Result> { - // Both sides of the path comparison are normalized: models tend to add - // `./` or backslashes, and request paths are matched under the same rules - // so a path that normalizes differently can never silently mismatch. - let mut request_paths = std::collections::BTreeSet::new(); - for file in &request.files { - if !request_paths.insert(normalize_path(&file.path)) { - bail!( - "Two conflicted paths normalize to the same value ({:?}); resolve this commit manually instead", - normalize_path(&file.path) - ); - } - } +/// A validated resolution for one hunk. Side picks stay symbolic so the +/// synthesis can copy the picked side's raw bytes instead of a re-rendered +/// string. +#[derive(Debug, Clone)] +pub(crate) enum HunkPick { + Ours, + Theirs, + Content(String), +} - let mut resolutions_by_path = std::collections::BTreeMap::new(); - for resolution in &response.resolutions { - let path = normalize_path(&resolution.path); - if resolutions_by_path - .insert(path.clone(), resolution) - .is_some() - { - bail!("The model returned more than one resolution for \"{path}\""); - } - } +/// Resolved hunks per file, aligned index-for-index with the request's files. +/// Keys are 0-based hunk indices; an empty map leaves the file's conflicts +/// untouched. +pub(crate) type PicksPerFile = Vec>; + +/// Validate caller-provided resolution specs against the request and translate +/// them into per-file hunk contents. Nothing is written to the repository here. +pub(crate) fn validate_specs( + request: &ResolutionRequest, + specs: &[ResolutionSpec], +) -> anyhow::Result { + let files_by_path = index_files_by_path(request)?; + let mut picks: PicksPerFile = vec![BTreeMap::new(); request.files.len()]; - let mut validated = Vec::with_capacity(request.files.len()); - for file in &request.files { - let resolution = resolutions_by_path - .remove(&normalize_path(&file.path)) + for spec in specs { + let &file_index = files_by_path + .get(&normalize_path(&spec.path)) .with_context(|| { - format!( - "The model returned no resolution for the conflicted file \"{}\"", - file.path - ) + format!("\"{}\" is not a conflicted file of this commit", spec.path) })?; - if resolution.hunks.len() != file.hunks.len() { + let file = &request.files[file_index]; + if spec.hunk == 0 || spec.hunk > file.hunks.len() { bail!( - "The model returned {} resolved hunks for \"{}\" but the file has {} conflicts", - resolution.hunks.len(), + "\"{}\" has conflicts 1..{}, but conflict {} was addressed", file.path, - file.hunks.len() + file.hunks.len(), + spec.hunk ); } - if resolution.reasoning.trim().is_empty() { - bail!("The model returned no reasoning for \"{}\"", file.path); + let hunk_index = spec.hunk - 1; + let pick = match &spec.resolution { + HunkResolution::Ours => HunkPick::Ours, + HunkResolution::Theirs => HunkPick::Theirs, + HunkResolution::Content(content) => { + ensure_no_markers(content, &file.path)?; + HunkPick::Content(content.clone()) + } + }; + if picks[file_index].insert(hunk_index, pick).is_some() { + bail!( + "Conflict {} of \"{}\" was addressed more than once", + spec.hunk, + file.path + ); } - let hunks: Vec = resolution - .hunks - .iter() - .map(|hunk| hunk.resolved_content.clone()) - .collect(); - let resolved_text = splice_resolved_file(file, &hunks)?; - // The inputs were verified to be free of marker-shaped content when - // the request was built, so any marker-shaped line in the spliced - // output can only come from the model. - if let Some(marker) = resolved_text - .lines() - .find(|line| is_marker_shaped(line.strip_suffix('\r').unwrap_or(line))) + } + + Ok(picks) +} + +/// Map normalized request paths to file indices, rejecting collisions. +pub(crate) fn index_files_by_path( + request: &ResolutionRequest, +) -> anyhow::Result> { + let mut files_by_path = BTreeMap::new(); + for (index, file) in request.files.iter().enumerate() { + if files_by_path + .insert(normalize_path(&file.path), index) + .is_some() { bail!( - "The model's resolution for \"{}\" still contains a conflict marker ({marker:?})", - file.path + "Two conflicted paths normalize to the same value ({:?}); resolve this commit manually instead", + normalize_path(&file.path) ); } - validated.push(ValidatedFile { - resolved_text, - hunks, - reasoning: resolution.reasoning.clone(), - }); } + Ok(files_by_path) +} - if let Some(path) = resolutions_by_path.into_keys().next() { - bail!("The model returned a resolution for \"{path}\", which was not requested"); +/// Reject content that contains a conflict-marker-shaped line. +pub(crate) fn ensure_no_markers(content: &str, path: &str) -> anyhow::Result<()> { + if let Some(marker) = content + .lines() + .map(|line| line.strip_suffix('\r').unwrap_or(line)) + .find(|line| is_marker_shaped(line)) + { + bail!("The resolution for \"{path}\" contains a conflict marker ({marker:?})"); + } + Ok(()) +} + +/// Normalize a path for comparison: trim, backslashes to slashes, strip a +/// leading `./`, collapse duplicate slashes. Applied to both request and +/// caller-provided paths. +pub(crate) fn normalize_path(path: &str) -> String { + let mut normalized = path.trim().replace('\\', "/"); + while normalized.contains("//") { + normalized = normalized.replace("//", "/"); } + normalized + .strip_prefix("./") + .map(str::to_owned) + .unwrap_or(normalized) +} - Ok(validated) +#[derive(Debug, Clone, Copy)] +enum SideKind { + Ours, + Base, + Theirs, } -/// Replace each conflict block in the merged text with its resolved content. +/// Render one side's narrowed version of a conflicted file: resolved blocks +/// become the picked content, unresolved blocks keep this side's own lines. /// -/// Non-conflicted lines are copied byte-for-byte including their own line -/// terminators, so mixed-EOL files stay untouched outside conflict regions. -/// Only the inserted resolution lines use the file's dominant EOL style, and a -/// single trailing newline on a resolution is dropped since the block's own -/// terminator is preserved. -fn splice_resolved_file(file: &FileConflict, resolutions: &[String]) -> anyhow::Result { +/// Non-conflicted lines, unresolved side lines, and side picks are copied +/// byte-for-byte including their own terminators, so mixed-EOL files stay +/// untouched outside resolved regions. Inserted custom-content lines use the +/// file's dominant EOL, and a single trailing newline on such content is +/// dropped since the block's own terminator is preserved. +fn synthesize_side( + file: &FileConflict, + picks: &BTreeMap, + side: SideKind, +) -> anyhow::Result { let eol = if file.merged_text.contains("\r\n") { "\r\n" } else { @@ -129,39 +169,73 @@ fn splice_resolved_file(file: &FileConflict, resolutions: &[String]) -> anyhow:: .map(|line| strip_terminator(line)) .collect(); let blocks = scan_conflict_blocks(&stripped_lines); - if blocks.len() != resolutions.len() { + if blocks.len() != file.hunks.len() { bail!( - "BUG: found {} conflict blocks in \"{}\" while splicing, but validated {} resolutions", + "BUG: found {} conflict blocks in \"{}\" while applying, but the request has {} hunks", blocks.len(), file.path, - resolutions.len() + file.hunks.len() ); } let mut result = String::with_capacity(file.merged_text.len()); let mut line = 0; - for (block, resolution) in blocks.iter().zip(resolutions) { + for (index, block) in blocks.iter().enumerate() { for raw in &raw_lines[line..block.start] { result.push_str(raw); } - let resolution = resolution - .strip_suffix('\n') - .map(|r| r.strip_suffix('\r').unwrap_or(r)) - .unwrap_or(resolution); - if !resolution.is_empty() { - let block_is_terminated = raw_lines[block.end].ends_with('\n'); - for (index, resolved_line) in resolution - .split('\n') - .map(|l| l.strip_suffix('\r').unwrap_or(l)) - .enumerate() - { - if index > 0 { + if let Some(pick) = picks.get(&index) { + let resolution = match pick { + // The picked side's raw bytes, terminators included. + HunkPick::Ours => { + for raw in &raw_lines[block.ours.clone()] { + result.push_str(raw); + } + line = block.end + 1; + continue; + } + HunkPick::Theirs => { + for raw in &raw_lines[block.theirs.clone()] { + result.push_str(raw); + } + line = block.end + 1; + continue; + } + HunkPick::Content(content) => content, + }; + let resolution = resolution + .strip_suffix('\n') + .map(|r| r.strip_suffix('\r').unwrap_or(r)) + .unwrap_or(resolution); + if !resolution.is_empty() { + let block_is_terminated = raw_lines[block.end].ends_with('\n'); + for (index, resolved_line) in resolution + .split('\n') + .map(|l| l.strip_suffix('\r').unwrap_or(l)) + .enumerate() + { + if index > 0 { + result.push_str(eol); + } + result.push_str(resolved_line); + } + if block_is_terminated { result.push_str(eol); } - result.push_str(resolved_line); } - if block_is_terminated { - result.push_str(eol); + } else { + let side_range = match side { + SideKind::Ours => block.ours.clone(), + SideKind::Theirs => block.theirs.clone(), + SideKind::Base => block.base.clone().with_context(|| { + format!( + "A conflict in \"{}\" carries no common-ancestor section and cannot be narrowed. Resolve this commit manually instead.", + file.path + ) + })?, + }; + for raw in &raw_lines[side_range] { + result.push_str(raw); } } line = block.end + 1; @@ -177,46 +251,109 @@ fn strip_terminator(line: &str) -> &str { line.strip_suffix('\r').unwrap_or(line) } -/// Normalize a path for comparison: trim, backslashes to slashes, strip a -/// leading `./`, collapse duplicate slashes. Applied to both request and -/// response paths. -fn normalize_path(path: &str) -> String { - let mut normalized = path.trim().replace('\\', "/"); - while normalized.contains("//") { - normalized = normalized.replace("//", "/"); - } - normalized - .strip_prefix("./") - .map(str::to_owned) - .unwrap_or(normalized) -} - -/// Write the resolved blobs and tree, rewrite the conflicted commit into a -/// normal commit (resolved tree, stripped message, conflict header removed), -/// and rebase its descendants. +/// Apply `picks` to the conflicted commit: synthesize narrowed side trees, +/// re-merge them, and rewrite the commit — into a normal commit when nothing +/// is left unresolved, or into a conflicted commit with the remaining +/// conflicts otherwise. Descendants are rebased either way. /// -/// Returns the rewritten commit's final id and the resulting workspace state. +/// Returns the rewritten commit's final id, the conflicts that remain per +/// file, and the resulting workspace state. pub(crate) fn apply( ctx: &mut but_ctx::Context, request: &ResolutionRequest, - validated: &[ValidatedFile], + picks_per_file: &PicksPerFile, dry_run: DryRun, perm: &mut RepoExclusive, -) -> anyhow::Result<(gix::ObjectId, WorkspaceState)> { +) -> anyhow::Result { let mut meta = ctx.meta()?; let (repo, mut ws, _) = ctx.workspace_mut_and_db_with_perm(perm)?; - let mut tree_editor = repo.edit_tree(request.merged_tree_id)?; - for (file, resolution) in request.files.iter().zip(validated) { - let blob_id = repo.write_blob(resolution.resolved_text.as_bytes())?; - tree_editor.upsert(file.rela_path.as_bstr(), file.entry_kind, blob_id)?; + // Narrow the sides: every resolved hunk's content goes into all three + // trees, so the re-merge below agrees there and conflicts only at what + // remains unresolved. + let mut base_edit = repo.edit_tree(request.base_tree_id)?; + let mut ours_edit = repo.edit_tree(request.ours_tree_id)?; + let mut theirs_edit = repo.edit_tree(request.theirs_tree_id)?; + for (file, picks) in request.files.iter().zip(picks_per_file) { + if picks.is_empty() { + continue; + } + for (tree, side) in [ + (&mut base_edit, SideKind::Base), + (&mut ours_edit, SideKind::Ours), + (&mut theirs_edit, SideKind::Theirs), + ] { + let content = synthesize_side(file, picks, side)?; + let blob_id = repo.write_blob(content.as_bytes())?; + tree.upsert(file.rela_path.as_bstr(), file.entry_kind, blob_id)?; + } + } + let base_tree_id = base_edit.write()?.detach(); + let ours_tree_id = ours_edit.write()?.detach(); + let theirs_tree_id = theirs_edit.write()?.detach(); + + // Auto-resolve like the rebase engine does when it writes conflicted + // commits: favor *ours* so the merged tree is clean content, never marker + // text, and detect the forcefully-resolved conflicts as unresolved. + use but_core::RepositoryExt as _; + let mut outcome = repo.merge_trees( + base_tree_id, + ours_tree_id, + theirs_tree_id, + repo.default_merge_labels(), + repo.merge_options_force_ours()?, + )?; + let merged_tree_id = outcome.tree.write()?.detach(); + let treat_as_unresolved = gix::merge::tree::TreatAsUnresolved::forced_resolution(); + let unresolved = outcome.has_unresolved_conflicts(treat_as_unresolved); + + let remaining: Vec = request + .files + .iter() + .zip(picks_per_file) + .filter_map(|(file, picks)| { + let remaining = file.hunks.len() - picks.len(); + (remaining > 0).then(|| RemainingConflicts { + path: file.path.clone(), + hunks: remaining, + }) + }) + .collect(); + if unresolved && remaining.is_empty() { + bail!( + "BUG: all conflicts of commit {} were resolved, yet re-merging the narrowed trees still conflicts", + request.commit_id + ); } - let resolved_tree_id = tree_editor.write()?.detach(); let mut editor = Editor::create(&mut ws, &mut meta, &repo)?; let (target_selector, mut commit) = editor.find_selectable_commit(request.commit_id)?; - commit.tree = resolved_tree_id; - commit.message = but_core::commit::strip_conflict_markers(commit.message.as_ref()); + if unresolved { + // Still conflicted: same commit shape the rebase engine writes, with + // the narrowed trees. Adding message markers is idempotent, and covers + // commits that were only marked by the legacy header — the header is + // cleared below, so the message must carry the state. + commit.message = but_core::commit::add_conflict_markers(commit.message.as_ref()); + let conflict_entries = but_core::commit::conflict_entries_from_merge_outcome( + &repo, + merged_tree_id, + &outcome, + treat_as_unresolved, + )?; + let tree_expression = TreeExpression { + base_tree_ids: vec![base_tree_id], + side_tree_ids: [ours_tree_id, theirs_tree_id].into_iter().collect(), + }; + commit.tree = but_core::commit::write_conflicted_tree( + &repo, + merged_tree_id, + &tree_expression, + &conflict_entries, + )?; + } else { + commit.tree = merged_tree_id; + commit.message = but_core::commit::strip_conflict_markers(commit.message.as_ref()); + } if let Some(headers) = Headers::try_from_commit(&commit) { Headers { conflicted: None, @@ -231,7 +368,11 @@ pub(crate) fn apply( let new_commit = rebase.lookup_pick(target_selector)?; let workspace = WorkspaceState::from_successful_rebase(rebase, &repo, dry_run)?; - Ok((new_commit, workspace)) + Ok(AppliedResolution { + new_commit, + remaining, + workspace, + }) } #[cfg(test)] @@ -239,6 +380,10 @@ mod tests { use super::*; use crate::resolve::context::split_lines; + const OURS: &str = "<<<<<<< gitbutler-resolve-ours"; + const BASE: &str = "||||||| gitbutler-resolve-base"; + const THEIRS: &str = ">>>>>>> gitbutler-resolve-theirs"; + fn test_file(merged_text: &str) -> FileConflict { let lines = split_lines(merged_text); let blocks = scan_conflict_blocks(&lines); @@ -249,92 +394,195 @@ mod tests { merged_text: merged_text.to_owned(), hunks: blocks .iter() - .map(|_| super::super::context::ConflictHunk { + .map(|block| super::super::context::ConflictHunk { context_before: String::new(), - ours: String::new(), - base: None, - theirs: String::new(), + ours: lines[block.ours.clone()].join("\n"), + base: block.base.clone().map(|range| lines[range].join("\n")), + theirs: lines[block.theirs.clone()].join("\n"), context_after: String::new(), }) .collect(), } } + fn picks(entries: &[(usize, &str)]) -> BTreeMap { + entries + .iter() + .map(|(index, content)| (*index, HunkPick::Content(content.to_string()))) + .collect() + } + + fn two_hunks() -> FileConflict { + test_file(&format!( + "start\n{OURS}\nours one\n{BASE}\nbase one\n=======\ntheirs one\n{THEIRS}\nmiddle\n{OURS}\nours two\n{BASE}\nbase two\n=======\ntheirs two\n{THEIRS}\nend\n" + )) + } + #[test] - fn splice_replaces_blocks_and_preserves_surroundings() { - let file = test_file( - "before\n<<<<<<< gitbutler-resolve-ours\na\n=======\nb\n>>>>>>> gitbutler-resolve-theirs\nbetween\n<<<<<<< gitbutler-resolve-ours\nc\n=======\nd\n>>>>>>> gitbutler-resolve-theirs\nafter\n", - ); - let result = - splice_resolved_file(&file, &["merged one".to_owned(), "merged two".to_owned()]) - .unwrap(); - assert_eq!(result, "before\nmerged one\nbetween\nmerged two\nafter\n"); + fn full_resolution_makes_all_sides_agree() { + let file = two_hunks(); + let all = picks(&[(0, "merged one"), (1, "merged two")]); + let expected = "start\nmerged one\nmiddle\nmerged two\nend\n"; + for side in [SideKind::Ours, SideKind::Base, SideKind::Theirs] { + assert_eq!(synthesize_side(&file, &all, side).unwrap(), expected); + } } #[test] - fn splice_empty_resolution_deletes_the_block() { - let file = test_file( - "before\n<<<<<<< gitbutler-resolve-ours\na\n=======\nb\n>>>>>>> gitbutler-resolve-theirs\nafter\n", + fn partial_resolution_keeps_each_sides_lines_for_unresolved_blocks() { + let file = two_hunks(); + let first_only = picks(&[(0, "merged one")]); + assert_eq!( + synthesize_side(&file, &first_only, SideKind::Ours).unwrap(), + "start\nmerged one\nmiddle\nours two\nend\n" + ); + assert_eq!( + synthesize_side(&file, &first_only, SideKind::Base).unwrap(), + "start\nmerged one\nmiddle\nbase two\nend\n" + ); + assert_eq!( + synthesize_side(&file, &first_only, SideKind::Theirs).unwrap(), + "start\nmerged one\nmiddle\ntheirs two\nend\n" ); - let result = splice_resolved_file(&file, &[String::new()]).unwrap(); - assert_eq!(result, "before\nafter\n"); } #[test] - fn splice_preserves_crlf() { - let file = test_file( - "a\r\n<<<<<<< gitbutler-resolve-ours\r\nx\r\n=======\r\ny\r\n>>>>>>> gitbutler-resolve-theirs\r\nb\r\n", - ); - let result = splice_resolved_file(&file, &["merged".to_owned()]).unwrap(); - assert_eq!(result, "a\r\nmerged\r\nb\r\n"); + fn empty_resolution_deletes_the_block() { + let file = test_file(&format!( + "before\n{OURS}\na\n{BASE}\nb\n=======\nc\n{THEIRS}\nafter\n" + )); + let result = synthesize_side(&file, &picks(&[(0, "")]), SideKind::Theirs).unwrap(); + assert_eq!(result, "before\nafter\n"); } - /// Mixed-EOL files must keep every non-conflicted line's own terminator — + /// Mixed-EOL files must keep every untouched line's own terminator — /// only inserted resolution lines use the dominant EOL. #[test] - fn splice_preserves_mixed_eol_outside_conflicts() { - let file = test_file( - "line1\nwin\r\nline3\n<<<<<<< gitbutler-resolve-ours\na\n=======\nb\n>>>>>>> gitbutler-resolve-theirs\nline4\n", - ); - let result = splice_resolved_file(&file, &["merged".to_owned()]).unwrap(); + fn synthesis_preserves_mixed_eol_outside_resolved_blocks() { + let file = test_file(&format!( + "line1\nwin\r\nline3\n{OURS}\na\n{BASE}\nb\n=======\nc\n{THEIRS}\nline4\n" + )); + let result = synthesize_side(&file, &picks(&[(0, "merged")]), SideKind::Ours).unwrap(); assert_eq!( result, "line1\nwin\r\nline3\nmerged\r\nline4\n", - "non-conflicted lines keep their own terminators; the inserted line uses the dominant (CRLF) EOL" + "untouched lines keep their own terminators; the inserted line uses the dominant (CRLF) EOL" ); } - /// A CRLF that only exists inside the replaced conflict block must not - /// change any line outside it. #[test] - fn splice_ignores_eol_of_deleted_block_content() { - let file = test_file( - "line1\n<<<<<<< gitbutler-resolve-ours\na\r\n=======\nb\n>>>>>>> gitbutler-resolve-theirs\nline2\n", - ); - let result = splice_resolved_file(&file, &[String::new()]).unwrap(); - assert_eq!(result, "line1\nline2\n"); + fn synthesis_strips_a_single_trailing_newline_from_resolutions() { + let file = test_file(&format!( + "before\n{OURS}\na\n{BASE}\nb\n=======\nc\n{THEIRS}\nafter\n" + )); + let result = synthesize_side(&file, &picks(&[(0, "merged\n")]), SideKind::Ours).unwrap(); + assert_eq!(result, "before\nmerged\nafter\n"); + let result = synthesize_side(&file, &picks(&[(0, "merged\n\n")]), SideKind::Ours).unwrap(); + assert_eq!(result, "before\nmerged\n\nafter\n"); + } + + #[test] + fn synthesis_without_trailing_newline_at_eof() { + let file = test_file(&format!( + "before\n{OURS}\na\n{BASE}\nb\n=======\nc\n{THEIRS}" + )); + let result = synthesize_side(&file, &picks(&[(0, "merged")]), SideKind::Ours).unwrap(); + assert_eq!(result, "before\nmerged"); + } + + fn test_request(files: Vec) -> ResolutionRequest { + ResolutionRequest { + commit_id: gix::ObjectId::null(gix::hash::Kind::Sha1), + commit_message: String::new(), + parent_message: None, + base_tree_id: gix::ObjectId::null(gix::hash::Kind::Sha1), + ours_tree_id: gix::ObjectId::null(gix::hash::Kind::Sha1), + theirs_tree_id: gix::ObjectId::null(gix::hash::Kind::Sha1), + files, + } + } + + fn spec(path: &str, hunk: usize, resolution: HunkResolution) -> ResolutionSpec { + ResolutionSpec { + path: path.into(), + hunk, + resolution, + } } #[test] - fn splice_strips_a_single_trailing_newline_from_resolutions() { - let file = test_file( - "before\n<<<<<<< gitbutler-resolve-ours\na\n=======\nb\n>>>>>>> gitbutler-resolve-theirs\nafter\n", + fn specs_translate_side_picks_and_content() { + let request = test_request(vec![two_hunks()]); + let picks = validate_specs( + &request, + &[ + spec("file.txt", 1, HunkResolution::Ours), + spec("./file.txt", 2, HunkResolution::Content("mixed".into())), + ], + ) + .unwrap(); + assert!(matches!(picks[0][&0], HunkPick::Ours)); + assert!(matches!(&picks[0][&1], HunkPick::Content(content) if content == "mixed")); + } + + /// A side pick must reproduce the picked side byte-for-byte — including a + /// trailing blank line and the side's own line terminators. + #[test] + fn side_picks_preserve_the_sides_raw_bytes() { + let file = test_file(&format!( + "start\n{OURS}\nfoo\n\n{BASE}\nbase\n=======\ntheirs one\r\ntheirs two\n{THEIRS}\nend\n" + )); + let ours: BTreeMap = [(0, HunkPick::Ours)].into_iter().collect(); + for side in [SideKind::Ours, SideKind::Base, SideKind::Theirs] { + assert_eq!( + synthesize_side(&file, &ours, side).unwrap(), + "start\nfoo\n\nend\n", + "the ours side's trailing blank line must survive" + ); + } + let theirs: BTreeMap = [(0, HunkPick::Theirs)].into_iter().collect(); + assert_eq!( + synthesize_side(&file, &theirs, SideKind::Ours).unwrap(), + "start\ntheirs one\r\ntheirs two\nend\n", + "the theirs side's own terminators must survive" ); - let result = splice_resolved_file(&file, &["merged\n".to_owned()]).unwrap(); - assert_eq!(result, "before\nmerged\nafter\n"); - // An intentional blank line (two newlines) keeps one. - let result = splice_resolved_file(&file, &["merged\n\n".to_owned()]).unwrap(); - assert_eq!(result, "before\nmerged\n\nafter\n"); } #[test] - fn splice_without_trailing_newline_at_eof() { - let file = test_file( - "before\n<<<<<<< gitbutler-resolve-ours\na\n=======\nb\n>>>>>>> gitbutler-resolve-theirs", + fn specs_are_validated() { + let request = test_request(vec![two_hunks()]); + let err = |specs: &[ResolutionSpec]| validate_specs(&request, specs).unwrap_err(); + + assert!( + err(&[spec("other.txt", 1, HunkResolution::Ours)]) + .to_string() + .contains("not a conflicted file") ); - let result = splice_resolved_file(&file, &["merged".to_owned()]).unwrap(); - assert_eq!( - result, "before\nmerged", - "an unterminated closing marker keeps the file unterminated" + assert!( + err(&[spec("file.txt", 3, HunkResolution::Ours)]) + .to_string() + .contains("has conflicts 1..2") + ); + assert!( + err(&[spec("file.txt", 0, HunkResolution::Ours)]) + .to_string() + .contains("has conflicts 1..2") + ); + assert!( + err(&[ + spec("file.txt", 1, HunkResolution::Ours), + spec("file.txt", 1, HunkResolution::Theirs), + ]) + .to_string() + .contains("more than once") + ); + assert!( + err(&[spec( + "file.txt", + 1, + HunkResolution::Content("<<<<<<< HEAD\nx".into()), + )]) + .to_string() + .contains("conflict marker") ); } diff --git a/crates/but-api/src/resolve/context.rs b/crates/but-api/src/resolve/context.rs index 187548ca7d0..1e49d6e279b 100644 --- a/crates/but-api/src/resolve/context.rs +++ b/crates/but-api/src/resolve/context.rs @@ -34,7 +34,9 @@ fn merge_labels() -> gix::merge::blob::builtin_driver::text::Labels<'static> { /// One conflicted region of a file, with the content of each side and a few /// lines of surrounding context. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, serde::Serialize)] +#[cfg_attr(feature = "export-schema", derive(schemars::JsonSchema))] +#[serde(rename_all = "camelCase")] pub struct ConflictHunk { /// Unconflicted lines directly before the conflict, clamped to the previous conflict. pub context_before: String, @@ -48,6 +50,9 @@ pub struct ConflictHunk { pub context_after: String, } +#[cfg(feature = "export-schema")] +but_schemars::register_sdk_type!(ConflictHunk); + /// A conflicted file together with its merged marker text and extracted hunks. #[derive(Debug, Clone)] pub struct FileConflict { @@ -72,8 +77,12 @@ pub struct ResolutionRequest { pub commit_message: String, /// The title of the commit's parent, i.e. the new base it was rebased onto. pub parent_message: Option, - /// The tree produced by re-merging the commit's conflict trees, with markers in blobs. - pub merged_tree_id: gix::ObjectId, + /// The commit's stored merge-base tree. + pub base_tree_id: gix::ObjectId, + /// The commit's stored *ours* tree, i.e. the new base it was rebased onto. + pub ours_tree_id: gix::ObjectId, + /// The commit's stored *theirs* tree, i.e. its own version. + pub theirs_tree_id: gix::ObjectId, /// All conflicted files, sorted by path. pub files: Vec, } @@ -106,9 +115,11 @@ pub fn build_request( .and_then(|parent_id| but_core::Commit::from_id(parent_id.attach(repo)).ok()) .map(|parent| commit_title(&parent)); + let (base, ours, theirs) = (base.detach(), ours.detach(), theirs.detach()); let repo = repo.clone().for_tree_diffing()?; // Merge without favoring a side to reproduce the actual conflicts, and - // force diff3-style markers so every hunk carries the common ancestor. + // force diff3-style markers with the sentinel labels so every hunk carries + // the common ancestor and marker lines are exactly known strings. let mut options: gix::merge::plumbing::tree::Options = repo.tree_merge_options()?.into(); options.blob_merge.text.conflict = gix::merge::blob::builtin_driver::text::Conflict::Keep { style: gix::merge::blob::builtin_driver::text::ConflictStyle::Diff3, @@ -213,7 +224,9 @@ pub fn build_request( commit_id, commit_message, parent_message, - merged_tree_id, + base_tree_id: base, + ours_tree_id: ours, + theirs_tree_id: theirs, files, }) } diff --git a/crates/but-api/src/resolve/mod.rs b/crates/but-api/src/resolve/mod.rs index 21cbca74559..b1ce8ad2549 100644 --- a/crates/but-api/src/resolve/mod.rs +++ b/crates/but-api/src/resolve/mod.rs @@ -1,22 +1,29 @@ -//! Resolve the conflicts of a conflicted commit with an LLM. +//! Inspect and resolve the conflicts of a conflicted commit — without edit mode. //! //! GitButler keeps conflicts as first-class committed state: a conflicted //! commit carries its merge inputs as trees. This module re-merges those trees -//! in memory, sends the conflict hunks to the configured LLM as a single-shot, -//! no-tools request, splices the returned per-hunk resolutions back -//! deterministically, and rewrites the commit into a normal one, rebasing its -//! descendants. The workspace never enters edit mode and the exclusive -//! worktree lock is held only for the final apply step — not while the model -//! is thinking. Disagreement with the result is handled by the oplog: the -//! operation records an undo point. - -use anyhow::Context as _; +//! in memory to expose the conflicts as per-file hunks +//! ([`commit_conflicts()`]), and applies per-hunk resolutions — a side pick or +//! custom content, for some or all hunks — by narrowing the conflict and +//! rewriting the commit, rebasing its descendants +//! ([`resolve_commit_conflict_hunks()`]). A partial resolution leaves the +//! commit conflicted with fewer conflicts, so callers can work incrementally. +//! +//! [`resolve_commit_conflicts_ai()`] is a client of the same core: it sends +//! the hunks to the configured LLM as a single-shot, no-tools request and +//! applies the returned full-coverage resolutions. In every case the workspace +//! never enters edit mode, the exclusive worktree lock is held only for the +//! apply step, and an oplog snapshot records an undo point. + +use std::collections::BTreeMap; + +use anyhow::{Context as _, bail}; use but_api_macros::but_api; use but_core::DryRun; use but_core::sync::RepoExclusive; use but_llm::{ChatMessage, LLMProvider}; use but_oplog::legacy::{OperationKind, SnapshotDetails}; -use serde::Serialize; +use serde::{Deserialize, Serialize}; use tracing::instrument; use crate::WorkspaceState; @@ -26,7 +33,173 @@ mod context; mod prompt; pub use context::{ConflictHunk, FileConflict, ResolutionRequest}; -pub use prompt::{FileResolution, HunkResolution, ResolutionResponse, SYSTEM_PROMPT}; +pub use prompt::{FileResolution, HunkContent, ResolutionResponse, SYSTEM_PROMPT}; + +/// The conflicts of a conflicted commit, as per-file hunks. +#[derive(Debug, Clone)] +pub struct CommitConflicts { + /// The conflicted commit. + pub commit_id: gix::ObjectId, + /// The conflicted files, sorted by path. + pub files: Vec, +} + +/// One conflicted file and its conflicts, in file order. +#[derive(Debug, Clone, Serialize)] +#[cfg_attr(feature = "export-schema", derive(schemars::JsonSchema))] +#[serde(rename_all = "camelCase")] +pub struct ConflictedFile { + /// The repo-relative path of the file. + pub path: String, + /// The conflicts of the file; hunks are addressed by their 1-based + /// position in this list. + pub hunks: Vec, +} + +#[cfg(feature = "export-schema")] +but_schemars::register_sdk_type!(ConflictedFile); + +/// Return the conflicts of the conflicted commit `commit_id` without entering +/// edit mode or touching the working tree. +/// +/// Hunks are identified by `(path, 1-based index)`; the extraction is +/// deterministic, so the same commit id always yields the same hunks and +/// [`resolve_commit_conflict_hunks()`] can be called with indices from this +/// result. Fails for commits whose conflicts have no hunk representation +/// (deletions/renames, binaries, oversized files, marker-like content) — those +/// need manual resolution in edit mode. +#[but_api(try_from = crate::resolve::json::CommitConflicts)] +#[instrument(err(Debug))] +pub fn commit_conflicts( + ctx: &but_ctx::Context, + commit_id: gix::ObjectId, +) -> anyhow::Result { + let _guard = ctx.shared_worktree_access(); + let repo = ctx.repo.get()?; + let request = context::build_request(&repo, commit_id)?; + Ok(CommitConflicts { + commit_id: request.commit_id, + files: request + .files + .into_iter() + .map(|file| ConflictedFile { + path: file.path, + hunks: file.hunks, + }) + .collect(), + }) +} + +/// How to resolve a single conflict hunk. +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "export-schema", derive(schemars::JsonSchema))] +#[serde(rename_all = "camelCase", tag = "type", content = "subject")] +pub enum HunkResolution { + /// Take the *ours* side, i.e. the new base the commit was rebased onto. + Ours, + /// Take the *theirs* side, i.e. the commit's own version. + Theirs, + /// Replace the conflict with this content (for mixed resolutions; an + /// empty string deletes the conflicted region). + Content(String), +} + +#[cfg(feature = "export-schema")] +but_schemars::register_sdk_type!(HunkResolution); + +/// One conflict to resolve, addressed by path and 1-based hunk index as +/// returned by [`commit_conflicts()`]. +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "export-schema", derive(schemars::JsonSchema))] +#[serde(rename_all = "camelCase")] +pub struct ResolutionSpec { + /// The repo-relative path of the conflicted file. + pub path: String, + /// The 1-based index of the conflict within the file. + pub hunk: usize, + /// How to resolve it. + pub resolution: HunkResolution, +} + +#[cfg(feature = "export-schema")] +but_schemars::register_sdk_type!(ResolutionSpec); + +/// Conflicts still unresolved in a file after an apply. +#[derive(Debug, Clone, Serialize)] +#[cfg_attr(feature = "export-schema", derive(schemars::JsonSchema))] +#[serde(rename_all = "camelCase")] +pub struct RemainingConflicts { + /// The repo-relative path of the file. + pub path: String, + /// How many conflicts remain in it. + pub hunks: usize, +} + +#[cfg(feature = "export-schema")] +but_schemars::register_sdk_type!(RemainingConflicts); + +/// The outcome of applying per-hunk resolutions to a conflicted commit. +#[derive(Debug, Clone)] +pub struct HunkResolutionResult { + /// The conflicted commit the resolutions were applied to. + pub commit_id: gix::ObjectId, + /// The rewritten commit. Still conflicted when `remaining` is non-empty. + pub new_commit: gix::ObjectId, + /// How many conflicts were resolved. + pub resolved: usize, + /// The conflicts that remain, per file. Empty when the commit is fully + /// resolved. + pub remaining: Vec, + /// Workspace state after the apply. + pub workspace: WorkspaceState, +} + +/// Apply `specs` to the conflicted commit `commit_id` and rebase descendants. +/// +/// Resolving a subset of the conflicts rewrites the commit into a conflicted +/// commit with only the remaining conflicts; resolving all of them rewrites it +/// into a normal commit. Either way the commit id changes — address follow-up +/// resolutions to the returned `new_commit`. An oplog snapshot records an undo +/// point. Nothing is written if any spec fails validation. +#[but_api(try_from = crate::resolve::json::HunkResolutionResult)] +#[instrument(skip(specs), err(Debug))] +pub fn resolve_commit_conflict_hunks( + ctx: &mut but_ctx::Context, + commit_id: gix::ObjectId, + specs: Vec, + dry_run: DryRun, +) -> anyhow::Result { + if specs.is_empty() { + bail!("No resolutions were provided"); + } + let request = { + let _guard = ctx.shared_worktree_access(); + let repo = ctx.repo.get()?; + context::build_request(&repo, commit_id)? + }; + let picks = apply::validate_specs(&request, &specs)?; + // Every spec addressed a distinct hunk, or validation would have failed. + let resolved = specs.len(); + + let mut guard = ctx.exclusive_worktree_access(); + ctx.invalidate_workspace_cache()?; + let applied = apply_with_snapshot( + ctx, + &request, + &picks, + OperationKind::ResolveConflicts, + dry_run, + guard.write_permission(), + )?; + + Ok(HunkResolutionResult { + commit_id: request.commit_id, + new_commit: applied.new_commit, + resolved, + remaining: applied.remaining, + workspace: applied.workspace, + }) +} /// How one conflicted file was resolved, for display to the user. #[derive(Debug, Clone, Serialize)] @@ -124,10 +297,10 @@ pub fn resolve_commit_conflicts_with( // lock, so it fails closed if the commit changed in the meantime. let attempt = || -> anyhow::Result<_> { let response = resolve(&request)?; - let validated = apply::validate(&request, &response)?; + let validated = validate_ai_response(&request, &response)?; Ok((validated, response.summary)) }; - let (validated, summary) = match attempt() { + let ((picks, files), summary) = match attempt() { Ok(outcome) => outcome, Err(first_failure) => { tracing::warn!( @@ -143,57 +316,111 @@ pub fn resolve_commit_conflicts_with( // call; the commit-presence check in apply must run against the state // under this exclusive lock, not a stale cache. ctx.invalidate_workspace_cache()?; - finish_with_perm( + let applied = apply_with_snapshot( ctx, &request, - validated, - summary, + &picks, + OperationKind::ResolveConflictsAi, dry_run, guard.write_permission(), - ) + )?; + + Ok(AiResolutionResult { + commit_id: request.commit_id, + new_commit: applied.new_commit, + summary, + files, + workspace: applied.workspace, + }) +} + +/// Check the model response against the request and translate it into +/// full-coverage picks plus the per-file display payload. Any mismatch fails +/// validation so the caller can retry the model once before giving up. +fn validate_ai_response( + request: &ResolutionRequest, + response: &ResolutionResponse, +) -> anyhow::Result<(apply::PicksPerFile, Vec)> { + let files_by_path = apply::index_files_by_path(request)?; + let mut picks: apply::PicksPerFile = vec![BTreeMap::new(); request.files.len()]; + let mut resolved_files: Vec> = vec![None; request.files.len()]; + + for resolution in &response.resolutions { + let Some(&index) = files_by_path.get(&apply::normalize_path(&resolution.path)) else { + bail!( + "The model returned a resolution for \"{}\", which was not requested", + resolution.path + ); + }; + if resolved_files[index].is_some() { + bail!( + "The model returned more than one resolution for \"{}\"", + resolution.path + ); + } + let file = &request.files[index]; + if resolution.hunks.len() != file.hunks.len() { + bail!( + "The model returned {} resolved hunks for \"{}\" but the file has {} conflicts", + resolution.hunks.len(), + file.path, + file.hunks.len() + ); + } + if resolution.reasoning.trim().is_empty() { + bail!("The model returned no reasoning for \"{}\"", file.path); + } + for (hunk_index, hunk) in resolution.hunks.iter().enumerate() { + apply::ensure_no_markers(&hunk.resolved_content, &file.path)?; + picks[index].insert( + hunk_index, + apply::HunkPick::Content(hunk.resolved_content.clone()), + ); + } + resolved_files[index] = Some(ResolvedFile { + path: file.path.clone(), + hunks: resolution + .hunks + .iter() + .map(|hunk| hunk.resolved_content.clone()) + .collect(), + reasoning: resolution.reasoning.clone(), + }); + } + + if let Some(missing) = resolved_files.iter().position(Option::is_none) { + bail!( + "The model returned no resolution for the conflicted file \"{}\"", + request.files[missing].path + ); + } + + Ok((picks, resolved_files.into_iter().flatten().collect())) } -fn finish_with_perm( +/// Record an undo point, apply `picks`, and commit the snapshot on success. +fn apply_with_snapshot( ctx: &mut but_ctx::Context, request: &ResolutionRequest, - validated: Vec, - summary: Option, + picks: &apply::PicksPerFile, + operation: OperationKind, dry_run: DryRun, perm: &mut RepoExclusive, -) -> anyhow::Result { +) -> anyhow::Result { let maybe_oplog_entry = but_oplog::UnmaterializedOplogSnapshot::from_details_with_perm( ctx, - SnapshotDetails::new(OperationKind::ResolveConflictsAi), + SnapshotDetails::new(operation), perm.read_permission(), dry_run, ); - let res = apply::apply(ctx, request, &validated, dry_run, perm); + let res = apply::apply(ctx, request, picks, dry_run, perm); if let Some(snapshot) = maybe_oplog_entry && res.is_ok() { snapshot.commit(ctx, perm).ok(); } - let (new_commit, workspace) = res?; - - let files = request - .files - .iter() - .zip(validated) - .map(|(file, validated)| ResolvedFile { - path: file.path.clone(), - hunks: validated.hunks, - reasoning: validated.reasoning, - }) - .collect(); - - Ok(AiResolutionResult { - commit_id: request.commit_id, - new_commit, - summary, - files, - workspace, - }) + res } /// JSON transport types for this module. @@ -202,6 +429,76 @@ pub mod json { use crate::json::HexHash; + /// JSON transport type for the conflicts of a conflicted commit. + #[derive(Debug, Serialize)] + #[cfg_attr(feature = "export-schema", derive(schemars::JsonSchema))] + #[serde(rename_all = "camelCase")] + pub struct CommitConflicts { + /// The conflicted commit. + #[cfg_attr(feature = "export-schema", schemars(with = "String"))] + pub commit_id: HexHash, + /// The conflicted files, sorted by path. + pub files: Vec, + } + + #[cfg(feature = "export-schema")] + but_schemars::register_sdk_type!(CommitConflicts); + + impl TryFrom for CommitConflicts { + type Error = anyhow::Error; + + fn try_from(value: super::CommitConflicts) -> Result { + let super::CommitConflicts { commit_id, files } = value; + Ok(Self { + commit_id: commit_id.into(), + files, + }) + } + } + + /// JSON transport type for the outcome of applying per-hunk resolutions. + #[derive(Debug, Serialize)] + #[cfg_attr(feature = "export-schema", derive(schemars::JsonSchema))] + #[serde(rename_all = "camelCase")] + pub struct HunkResolutionResult { + /// The conflicted commit the resolutions were applied to. + #[cfg_attr(feature = "export-schema", schemars(with = "String"))] + pub commit_id: HexHash, + /// The rewritten commit. Still conflicted when `remaining` is non-empty. + #[cfg_attr(feature = "export-schema", schemars(with = "String"))] + pub new_commit: HexHash, + /// How many conflicts were resolved. + pub resolved: usize, + /// The conflicts that remain, per file. + pub remaining: Vec, + /// Workspace state after the apply. + pub workspace: crate::json::WorkspaceState, + } + + #[cfg(feature = "export-schema")] + but_schemars::register_sdk_type!(HunkResolutionResult); + + impl TryFrom for HunkResolutionResult { + type Error = anyhow::Error; + + fn try_from(value: super::HunkResolutionResult) -> Result { + let super::HunkResolutionResult { + commit_id, + new_commit, + resolved, + remaining, + workspace, + } = value; + Ok(Self { + commit_id: commit_id.into(), + new_commit: new_commit.into(), + resolved, + remaining, + workspace: workspace.try_into()?, + }) + } + } + /// JSON transport type for the outcome of an AI conflict resolution. #[derive(Debug, Serialize)] #[cfg_attr(feature = "export-schema", derive(schemars::JsonSchema))] diff --git a/crates/but-api/src/resolve/prompt.rs b/crates/but-api/src/resolve/prompt.rs index 041df75d550..91d1972166d 100644 --- a/crates/but-api/src/resolve/prompt.rs +++ b/crates/but-api/src/resolve/prompt.rs @@ -86,7 +86,7 @@ pub struct FileResolution { description = "An ordered array with one entry per conflict in the file, matching the 'Conflict 1 of N', 'Conflict 2 of N' order from the input." )] /// The per-conflict resolutions, in input order. - pub hunks: Vec, + pub hunks: Vec, #[schemars( description = "Terse, direct prose — enough detail to verify the decision, not a wall of text. State what each side did in this file, what you kept, and any trade-off. Typically 1-4 sentences." )] @@ -97,7 +97,7 @@ pub struct FileResolution { /// The replacement content for a single conflict block. #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] #[serde(rename_all = "camelCase")] -pub struct HunkResolution { +pub struct HunkContent { #[schemars( description = "ONLY the merged content that replaces this specific conflict block (the region between <<<<<<< and >>>>>>>). Do NOT include surrounding non-conflicted code — the application splices each resolution into the original file automatically. To accept one side entirely, return that side's content verbatim. For an intentional deletion, use an empty string." )] diff --git a/crates/but-api/tests/api/main.rs b/crates/but-api/tests/api/main.rs index 227c379f940..6c7d27fc3fa 100644 --- a/crates/but-api/tests/api/main.rs +++ b/crates/but-api/tests/api/main.rs @@ -1,4 +1,5 @@ mod branch_apply; mod branch_checkout; mod resolve_ai; +mod resolve_hunks; mod support; diff --git a/crates/but-api/tests/api/resolve_ai.rs b/crates/but-api/tests/api/resolve_ai.rs index 70d0b8b702f..8b4ca6a3cf4 100644 --- a/crates/but-api/tests/api/resolve_ai.rs +++ b/crates/but-api/tests/api/resolve_ai.rs @@ -1,6 +1,6 @@ use anyhow::Result; use but_api::resolve::{ - FileResolution, HunkResolution, ResolutionResponse, resolve_commit_conflicts_with, + FileResolution, HunkContent, ResolutionResponse, resolve_commit_conflicts_with, }; use but_core::DryRun; use gitbutler_oplog::OplogExt as _; @@ -14,15 +14,18 @@ fn conflicted_context() -> Result<(but_ctx::Context, gix::ObjectId, tempfile::Te Ok((ctx, conflicted_commit, tmp)) } -fn merged_response(path: &str, content: &str) -> ResolutionResponse { +fn merged_response(path: &str, contents: &[&str]) -> ResolutionResponse { ResolutionResponse { - summary: Some("### Conflicting changes\nBoth sides changed line two.".into()), + summary: Some("### Conflicting changes\nBoth sides changed lines two and six.".into()), resolutions: vec![FileResolution { path: path.into(), - hunks: vec![HunkResolution { - resolved_content: content.into(), - }], - reasoning: "Combined both changes of line two.".into(), + hunks: contents + .iter() + .map(|content| HunkContent { + resolved_content: (*content).into(), + }) + .collect(), + reasoning: "Combined both sides of each change.".into(), }], } } @@ -36,7 +39,7 @@ fn resolves_conflicted_commit_and_rebases_descendants() -> Result<()> { assert_eq!(request.files.len(), 1, "one conflicted file expected"); let file = &request.files[0]; assert_eq!(file.path, "conflict"); - assert_eq!(file.hunks.len(), 1, "one conflict hunk expected"); + assert_eq!(file.hunks.len(), 2, "two conflict hunks expected"); let hunk = &file.hunks[0]; assert_eq!(hunk.ours, "line two changed by the new base"); assert_eq!(hunk.theirs, "line two changed by this commit"); @@ -45,10 +48,14 @@ fn resolves_conflicted_commit_and_rebases_descendants() -> Result<()> { Some("line two"), "diff3 markers should carry the common ancestor" ); + assert_eq!(file.hunks[1].ours, "line six changed by the new base"); assert!(request.commit_message.contains("Change line two")); Ok(merged_response( "conflict", - "line two changed by both sides", + &[ + "line two changed by both sides", + "line six changed by both sides", + ], )) })?; @@ -73,7 +80,7 @@ fn resolves_conflicted_commit_and_rebases_descendants() -> Result<()> { .object()?; assert_eq!( resolved_blob.data.as_slice(), - b"line one\nline two changed by both sides\nline three\n" + b"line one\nline two changed by both sides\nline three\nline four\nline five\nline six changed by both sides\nline seven\n" ); let untouched_blob = repo .rev_parse_single(format!("{}:file", result.new_commit).as_str())? @@ -99,7 +106,7 @@ fn resolves_conflicted_commit_and_rebases_descendants() -> Result<()> { .object()?; assert_eq!( descendant_conflict.data.as_slice(), - b"line one\nline two changed by both sides\nline three\n", + b"line one\nline two changed by both sides\nline three\nline four\nline five\nline six changed by both sides\nline seven\n", "the descendant must inherit the resolution" ); let descendant_later = repo @@ -135,14 +142,14 @@ fn invalid_response_is_retried_once_then_fails_without_changes() -> Result<()> { let err = resolve_commit_conflicts_with(&mut ctx, conflicted_commit, DryRun::No, |_request| { calls.set(calls.get() + 1); // Wrong path: never matches the requested file. - Ok(merged_response("wrong-path", "content")) + Ok(merged_response("wrong-path", &["content", "content"])) }) .unwrap_err(); assert_eq!(calls.get(), 2, "the model must be retried exactly once"); assert!( - err.to_string().contains("conflict"), - "error should name the file: {err}" + err.to_string().contains("wrong-path"), + "error should name the unrequested path: {err}" ); let repo = ctx.repo.get()?; @@ -161,7 +168,10 @@ fn resolution_with_leaked_markers_is_rejected() -> Result<()> { let err = resolve_commit_conflicts_with(&mut ctx, conflicted_commit, DryRun::No, |_request| { Ok(merged_response( "conflict", - "<<<<<<< ours\nstill conflicted\n=======\noops\n>>>>>>> theirs", + &[ + "<<<<<<< ours\nstill conflicted\n=======\noops\n>>>>>>> theirs", + "fine", + ], )) }) .unwrap_err(); diff --git a/crates/but-api/tests/api/resolve_hunks.rs b/crates/but-api/tests/api/resolve_hunks.rs new file mode 100644 index 00000000000..aa6ddeb724f --- /dev/null +++ b/crates/but-api/tests/api/resolve_hunks.rs @@ -0,0 +1,209 @@ +use anyhow::Result; +use but_api::resolve::{ + HunkResolution, ResolutionSpec, commit_conflicts, resolve_commit_conflict_hunks, +}; +use but_core::DryRun; +use gitbutler_oplog::OplogExt as _; +use gix::prelude::ObjectIdExt as _; + +fn conflicted_context() -> Result<(but_ctx::Context, gix::ObjectId, tempfile::TempDir)> { + let (repo, tmp) = crate::support::writable_scenario("resolve-ai-conflicted-commit"); + crate::support::persist_default_target(&repo)?; + let conflicted_commit = repo.rev_parse_single("refs/tags/conflicted")?.detach(); + let ctx = but_ctx::Context::from_repo(repo)?.with_memory_app_cache(); + Ok((ctx, conflicted_commit, tmp)) +} + +fn spec(path: &str, hunk: usize, resolution: HunkResolution) -> ResolutionSpec { + ResolutionSpec { + path: path.into(), + hunk, + resolution, + } +} + +#[test] +fn conflicts_are_listed_without_entering_edit_mode() -> Result<()> { + let (ctx, conflicted_commit, _tmp) = conflicted_context()?; + + let conflicts = commit_conflicts(&ctx, conflicted_commit)?; + assert_eq!(conflicts.commit_id, conflicted_commit); + assert_eq!(conflicts.files.len(), 1); + let file = &conflicts.files[0]; + assert_eq!(file.path, "conflict"); + assert_eq!(file.hunks.len(), 2); + assert_eq!(file.hunks[0].ours, "line two changed by the new base"); + assert_eq!(file.hunks[0].theirs, "line two changed by this commit"); + assert_eq!(file.hunks[0].base.as_deref(), Some("line two")); + assert_eq!(file.hunks[0].context_before, "line one"); + assert_eq!(file.hunks[1].ours, "line six changed by the new base"); + assert_eq!(file.hunks[1].context_after, "line seven\n"); + + // Read-only: the commit and the workspace are untouched. + let repo = ctx.repo.get()?; + let commit = but_core::Commit::from_id(conflicted_commit.attach(&repo))?; + assert!(commit.is_conflicted()); + Ok(()) +} + +#[test] +fn partial_resolution_narrows_the_conflict_then_completes() -> Result<()> { + let (mut ctx, conflicted_commit, _tmp) = conflicted_context()?; + + // Resolve only the first of the two conflicts, with mixed content. + let first = resolve_commit_conflict_hunks( + &mut ctx, + conflicted_commit, + vec![spec( + "conflict", + 1, + HunkResolution::Content("line two merged".into()), + )], + DryRun::No, + )?; + assert_eq!(first.resolved, 1); + assert_eq!(first.remaining.len(), 1); + assert_eq!(first.remaining[0].path, "conflict"); + assert_eq!(first.remaining[0].hunks, 1); + + { + let repo = ctx.repo.get()?; + let narrowed = but_core::Commit::from_id(first.new_commit.attach(&repo))?; + assert!( + narrowed.is_conflicted(), + "a partial resolution must leave the commit conflicted" + ); + assert_eq!( + narrowed + .message + .to_string() + .matches("GitButler-Conflict") + .count(), + 1, + "re-marking the message must be idempotent" + ); + // The auto-resolution carries the resolution and favors ours elsewhere. + let auto_blob = repo + .rev_parse_single(format!("{}:.auto-resolution/conflict", first.new_commit).as_str())? + .object()?; + assert_eq!( + auto_blob.data.as_slice(), + b"line one\nline two merged\nline three\nline four\nline five\nline six changed by the new base\nline seven\n" + ); + // The descendant sits on the narrowed commit. + let descendant = repo + .rev_parse_single("refs/heads/branchy")? + .object()? + .into_commit(); + assert_eq!( + descendant.decode()?.parents().next(), + Some(first.new_commit) + ); + } + + // The narrowed commit reports exactly the one remaining conflict. + let conflicts = commit_conflicts(&ctx, first.new_commit)?; + assert_eq!(conflicts.files.len(), 1); + let file = &conflicts.files[0]; + assert_eq!(file.hunks.len(), 1, "one conflict must remain"); + assert_eq!(file.hunks[0].ours, "line six changed by the new base"); + assert_eq!(file.hunks[0].theirs, "line six changed by this commit"); + assert_eq!( + file.hunks[0].base.as_deref(), + Some("line six"), + "the remaining conflict keeps its common ancestor" + ); + + // Resolve the remaining conflict by taking theirs — the commit normalizes. + let second = resolve_commit_conflict_hunks( + &mut ctx, + first.new_commit, + vec![spec("conflict", 1, HunkResolution::Theirs)], + DryRun::No, + )?; + assert_eq!(second.resolved, 1); + assert!(second.remaining.is_empty()); + + { + let repo = ctx.repo.get()?; + let resolved = but_core::Commit::from_id(second.new_commit.attach(&repo))?; + assert!(!resolved.is_conflicted()); + assert_eq!(resolved.message.to_string(), "Change line two"); + let blob = repo + .rev_parse_single(format!("{}:conflict", second.new_commit).as_str())? + .object()?; + assert_eq!( + blob.data.as_slice(), + b"line one\nline two merged\nline three\nline four\nline five\nline six changed by this commit\nline seven\n" + ); + let later = repo + .rev_parse_single("refs/heads/branchy:later")? + .object()?; + assert_eq!(later.data.as_slice(), b"descendant\n"); + } + + // Both applies recorded undo points. + let resolve_snapshots = ctx + .snapshots_iter(None, Vec::new(), None)? + .filter_map(Result::ok) + .filter(|snapshot| { + snapshot.details.as_ref().is_some_and(|details| { + matches!( + details.operation, + but_oplog::legacy::OperationKind::ResolveConflicts + ) + }) + }) + .count(); + assert_eq!(resolve_snapshots, 2); + + Ok(()) +} + +#[test] +fn taking_one_side_for_all_hunks_resolves_the_commit() -> Result<()> { + let (mut ctx, conflicted_commit, _tmp) = conflicted_context()?; + + let result = resolve_commit_conflict_hunks( + &mut ctx, + conflicted_commit, + vec![ + spec("conflict", 1, HunkResolution::Ours), + spec("conflict", 2, HunkResolution::Ours), + ], + DryRun::No, + )?; + assert_eq!(result.resolved, 2); + assert!(result.remaining.is_empty()); + + let repo = ctx.repo.get()?; + let resolved = but_core::Commit::from_id(result.new_commit.attach(&repo))?; + assert!(!resolved.is_conflicted()); + let blob = repo + .rev_parse_single(format!("{}:conflict", result.new_commit).as_str())? + .object()?; + assert_eq!( + blob.data.as_slice(), + b"line one\nline two changed by the new base\nline three\nline four\nline five\nline six changed by the new base\nline seven\n" + ); + Ok(()) +} + +#[test] +fn invalid_specs_change_nothing() -> Result<()> { + let (mut ctx, conflicted_commit, _tmp) = conflicted_context()?; + + let err = resolve_commit_conflict_hunks( + &mut ctx, + conflicted_commit, + vec![spec("conflict", 3, HunkResolution::Ours)], + DryRun::No, + ) + .unwrap_err(); + assert!(err.to_string().contains("has conflicts 1..2"), "{err}"); + + let repo = ctx.repo.get()?; + let commit = but_core::Commit::from_id(conflicted_commit.attach(&repo))?; + assert!(commit.is_conflicted(), "nothing may be written on failure"); + Ok(()) +} diff --git a/crates/but-api/tests/fixtures/scenario/resolve-ai-conflicted-commit.sh b/crates/but-api/tests/fixtures/scenario/resolve-ai-conflicted-commit.sh index ecf2a111459..0706616d151 100755 --- a/crates/but-api/tests/fixtures/scenario/resolve-ai-conflicted-commit.sh +++ b/crates/but-api/tests/fixtures/scenario/resolve-ai-conflicted-commit.sh @@ -24,12 +24,13 @@ EOF # inputs are kept as trees, the commit tree is auto-resolved favoring "ours", # and the conflict is recorded in the message trailer plus the legacy header. # -# The content conflict: base, ours (the new base), and theirs (the commit's own -# version) each have a different middle line in "conflict". +# The content conflicts: base, ours (the new base), and theirs (the commit's +# own version) each differ at "line two" and at "line six" of "conflict", +# far enough apart to form two separate conflict hunks. unrelated_blob=$(git rev-parse HEAD:file) -base_blob=$(printf "line one\nline two\nline three\n" | git hash-object -wt blob --stdin) -ours_blob=$(printf "line one\nline two changed by the new base\nline three\n" | git hash-object -wt blob --stdin) -theirs_blob=$(printf "line one\nline two changed by this commit\nline three\n" | git hash-object -wt blob --stdin) +base_blob=$(printf "line one\nline two\nline three\nline four\nline five\nline six\nline seven\n" | git hash-object -wt blob --stdin) +ours_blob=$(printf "line one\nline two changed by the new base\nline three\nline four\nline five\nline six changed by the new base\nline seven\n" | git hash-object -wt blob --stdin) +theirs_blob=$(printf "line one\nline two changed by this commit\nline three\nline four\nline five\nline six changed by this commit\nline seven\n" | git hash-object -wt blob --stdin) conflict_files_blob=$(git hash-object -wt blob --stdin <` — enter resolution mode. This puts conflict markers in the files. -3. **Read the conflicted files** to see the `<<<<<<<` / `=======` / `>>>>>>>` markers. -4. **Edit the files** to resolve conflicts by choosing the correct content and removing markers. -5. `but resolve finish` — finalize. Do NOT run this without editing the files first. -6. Repeat for any remaining conflicted commits. +1. `but resolve conflicts ` — shows the conflicts of that branch's oldest conflicted commit, numbered per file. Each conflict has three sections: `ours` (the new base the commit was rebased onto), `base` (common ancestor), and `theirs` (the commit's own version). Without a branch it picks the first conflicted branch and tells you which; the output also says when other conflicted commits exist. +2. Apply one resolution per command: + - Merged/mixed content (the common case — combine the intent of both sides): pipe it to `but resolve apply :` via stdin (heredoc) or pass `--file `. The content replaces the whole conflicted region; never include conflict markers. + - Take a side entirely: `but resolve apply : --ours` or `--theirs` (bare `` applies the side to all conflicts in that file). + - `apply` targets the same default commit as `conflicts`; when more than one branch is conflicted, pin it with `--commit `. +3. Go back to step 1 until it reports no conflicts. Commit ids are not stable here — every apply rewrites the commit — but branch names are, so address everything by branch and never bookkeep commit ids. Partial progress is fine: a commit with unresolved conflicts left stays `{conflicted}` with exactly those conflicts. -**Common mistakes:** Do NOT use `but amend` on conflicted commits (it won't work). Do NOT skip step 4 — you must actually edit the files to remove conflict markers before finishing. +A wrong resolution is reverted with `but undo`. + +`but resolve --ai` resolves a whole commit with the configured AI model in one shot, and `but resolve --ai` without a commit resolves every conflicted commit, oldest first — acceptable as a fallback, but you usually have better context than that model does, so prefer resolving the conflicts yourself with `apply`. + +**Use edit mode only when you must run code against the resolution** (build/tests at that commit): `but resolve ` checks the conflicted commit out with markers in the files, then edit the files, then `but resolve finish` (or `cancel`). Do NOT use `but amend` on conflicted commits (it won't work), and never run `finish` while markers remain in the files. ## Git-to-But Map diff --git a/crates/but/src/args/resolve.rs b/crates/but/src/args/resolve.rs index 2ee97527b2f..50000680ec1 100644 --- a/crates/but/src/args/resolve.rs +++ b/crates/but/src/args/resolve.rs @@ -1,5 +1,43 @@ #[derive(Debug, clap::Subcommand)] pub enum Subcommands { + /// List the conflicts of a conflicted commit, without entering resolution mode. + /// + /// Each conflict is shown with its ours side (the new base the commit was + /// rebased onto), the common ancestor, and its theirs side (the commit's + /// own version), numbered per file for use with `but resolve apply`. + Conflicts { + /// A conflicted commit, or a branch (meaning its oldest conflicted commit). + /// Defaults to the first conflicted branch's oldest conflicted commit. + commit: Option, + }, + + /// Resolve conflicts of a conflicted commit, without entering resolution mode. + /// + /// Targets one conflict (`:`, numbers from `but resolve conflicts`) + /// or every conflict in a file (`` with `--ours`/`--theirs`). The + /// replacement content for mixed resolutions is read from `--file` or stdin. + /// Resolving only some conflicts keeps the commit conflicted with the rest, + /// so conflicts can be worked off incrementally; the commit id changes with + /// every apply. Undo with `but undo`. + Apply { + /// The conflicted file, optionally with a 1-based conflict number (`:`). + target: String, + /// A conflicted commit, or a branch (meaning its oldest conflicted + /// commit — branch names stay stable across applies, unlike commit ids). + /// Defaults to the first conflicted branch's oldest conflicted commit. + #[clap(long)] + commit: Option, + /// Take the ours side: the new base the commit was rebased onto. + #[clap(long, conflicts_with_all = ["theirs", "file"])] + ours: bool, + /// Take the theirs side: the commit's own version. + #[clap(long, conflicts_with = "file")] + theirs: bool, + /// Read the replacement content from this file (otherwise from stdin). + #[clap(long, short = 'F')] + file: Option, + }, + /// Show the status of conflict resolution, listing remaining conflicted files. Status, diff --git a/crates/but/src/command/legacy/oplog.rs b/crates/but/src/command/legacy/oplog.rs index 187c526f6da..85c4449eab6 100644 --- a/crates/but/src/command/legacy/oplog.rs +++ b/crates/but/src/command/legacy/oplog.rs @@ -165,6 +165,7 @@ pub(crate) fn show_oplog( | OperationKind::MoveCommitFile | OperationKind::FileChanges | OperationKind::EnterEditMode + | OperationKind::ResolveConflicts | OperationKind::ResolveConflictsAi | OperationKind::SyncWorkspace | OperationKind::CreateDependentBranch @@ -189,6 +190,7 @@ pub(crate) fn show_oplog( OperationKind::CreateCommit => t.success.paint(operation_type.kind_str()), OperationKind::UpdateCommitMessage | OperationKind::AmendCommit + | OperationKind::ResolveConflicts | OperationKind::ResolveConflictsAi => t.attention.paint(operation_type.kind_str()), OperationKind::UndoCommit | OperationKind::RestoreFromSnapshot diff --git a/crates/but/src/command/legacy/resolve.rs b/crates/but/src/command/legacy/resolve.rs index bd25aeb9751..93f21c31171 100644 --- a/crates/but/src/command/legacy/resolve.rs +++ b/crates/but/src/command/legacy/resolve.rs @@ -36,6 +36,14 @@ pub(crate) fn handle( return resolve_with_ai(ctx, out, commit_id.as_deref()); } match cmd { + Some(Subcommands::Conflicts { commit }) => list_conflicts(ctx, out, commit.as_deref()), + Some(Subcommands::Apply { + target, + commit, + ours, + theirs, + file, + }) => apply_resolutions(ctx, out, commit.as_deref(), &target, ours, theirs, file), Some(Subcommands::Status) => show_status(ctx, out), Some(Subcommands::Finish) => finish_resolution(ctx, out), Some(Subcommands::Cancel { force }) => cancel_resolution(ctx, out, force), @@ -407,6 +415,348 @@ fn cancel_resolution(ctx: &mut Context, out: &mut OutputChannel, force: bool) -> Ok(()) } +/// Where a conflicted-commit target resolved to, with enough context to tell +/// the user what scope they are working in. +struct ConflictTarget { + commit_oid: gix::ObjectId, + /// The branch the commit sits on, if it is a known conflicted commit. + branch: Option, + /// Conflicted commits elsewhere in the workspace (other ids than the target). + other_conflicted: usize, +} + +/// Resolve `target` — a commit id, a CLI id, or a branch name (meaning that +/// branch's oldest conflicted commit) — or default to the oldest conflicted +/// commit of the first conflicted branch. `Ok(None)` means nothing is +/// conflicted. +fn target_conflicted_commit( + ctx: &mut Context, + target: Option<&str>, +) -> Result> { + let conflicts_by_branch = find_conflicted_commits(ctx)?; + let commit_oid = match target { + // An exact conflicted-branch name scopes to that branch. + Some(target) if conflicts_by_branch.contains_key(target) => { + conflicts_by_branch[target][0].commit_oid + } + Some(target) => match parse_commit_or_branch(ctx, target)? { + CommitOrBranch::Commit(commit_oid) => commit_oid, + CommitOrBranch::Branch(name) => match conflicts_by_branch.get(&name) { + Some(commits) => commits[0].commit_oid, + None => bail!("Branch \"{name}\" has no conflicted commits."), + }, + }, + None => match conflicts_by_branch.values().flatten().next() { + Some(commit) => commit.commit_oid, + None => return Ok(None), + }, + }; + + let branch = conflicts_by_branch + .iter() + .find(|(_, commits)| commits.iter().any(|commit| commit.commit_oid == commit_oid)) + .map(|(branch, _)| branch.clone()); + let other_conflicted = conflicts_by_branch + .values() + .flatten() + .filter(|commit| commit.commit_oid != commit_oid) + .map(|commit| commit.commit_oid) + .collect::>() + .len(); + Ok(Some(ConflictTarget { + commit_oid, + branch, + other_conflicted, + })) +} + +enum CommitOrBranch { + Commit(gix::ObjectId), + Branch(String), +} + +/// Resolve a user-provided identifier to a commit or a branch. +fn parse_commit_or_branch(ctx: &mut Context, target: &str) -> Result { + let id_map = IdMap::legacy_new_from_context(ctx, None)?; + let matches = id_map.parse_using_context(target, ctx)?; + match matches.as_slice() { + [] => bail!( + "\"{target}\" is neither a commit nor a branch. Try running 'but status' to see what is available." + ), + [CliId::Commit { commit_id, .. }] => Ok(CommitOrBranch::Commit(*commit_id)), + [CliId::Branch { name, .. }] => Ok(CommitOrBranch::Branch(name.clone())), + [_] => bail!("\"{target}\" does not refer to a commit or a branch"), + _ => bail!( + "\"{target}\" is ambiguous. Please provide more characters to uniquely identify it." + ), + } +} + +/// List the conflicts of a conflicted commit without entering resolution mode. +fn list_conflicts(ctx: &mut Context, out: &mut OutputChannel, commit: Option<&str>) -> Result<()> { + let t = theme::get(); + let Some(target) = target_conflicted_commit(ctx, commit)? else { + if let Some(human_out) = out.for_human() { + writeln!( + human_out, + "{}", + t.success.paint("No conflicted commits found.") + )?; + } + if let Some(json_out) = out.for_json() { + json_out.write_value(serde_json::json!({ "commit_id": null, "files": [] }))?; + } + return Ok(()); + }; + + let conflicts = but_api::resolve::commit_conflicts(ctx, target.commit_oid)?; + + if let Some(human_out) = out.for_human() { + let repo = ctx.repo.get()?; + writeln!( + human_out, + "{} {}{}", + t.important.paint("Conflicts in commit"), + t.commit_id + .paint(shorten_object_id(&repo, target.commit_oid)), + target + .branch + .as_deref() + .map(|branch| format!( + "{} {}", + t.important.paint(" on branch"), + t.local_branch.paint(branch) + )) + .unwrap_or_default() + )?; + for file in &conflicts.files { + writeln!(human_out)?; + writeln!(human_out, "{}", t.attention.paint(&file.path))?; + let total = file.hunks.len(); + for (index, hunk) in file.hunks.iter().enumerate() { + writeln!( + human_out, + "{}", + t.important + .paint(format!("── conflict {} of {total}", index + 1)) + )?; + writeln!(human_out, "{}", t.hint.paint("<<< ours (new base)"))?; + writeln!(human_out, "{}", hunk.ours)?; + if let Some(base) = &hunk.base { + writeln!(human_out, "{}", t.hint.paint("||| base"))?; + writeln!(human_out, "{base}")?; + } + writeln!(human_out, "{}", t.hint.paint(">>> theirs (this commit)"))?; + writeln!(human_out, "{}", hunk.theirs)?; + } + } + writeln!(human_out)?; + if target.other_conflicted > 0 { + writeln!( + human_out, + "{}", + t.attention.paint(format!( + "{} other conflicted commit{} exist{} — scope with `but resolve conflicts `.", + target.other_conflicted, + if target.other_conflicted == 1 { "" } else { "s" }, + if target.other_conflicted == 1 { "s" } else { "" }, + )) + )?; + } + writeln!( + human_out, + "{}", + t.hint.paint( + "Resolve with `but resolve apply [:] --ours|--theirs` or pipe mixed content into `but resolve apply :`. `but resolve --ai` resolves everything at once." + ) + )?; + } + + if let Some(json_out) = out.for_json() { + json_out.write_value(serde_json::json!({ + "commit_id": conflicts.commit_id.to_string(), + "branch": target.branch, + "files": conflicts.files, + }))?; + } + + Ok(()) +} + +/// Apply resolutions to one conflict (`:`) or to every conflict of a +/// file, without entering resolution mode. +fn apply_resolutions( + ctx: &mut Context, + out: &mut OutputChannel, + commit: Option<&str>, + target: &str, + ours: bool, + theirs: bool, + file: Option, +) -> Result<()> { + use but_api::resolve::{HunkResolution, ResolutionSpec}; + + let t = theme::get(); + let mode = operating_mode(ctx)?.operating_mode; + if matches!(mode, OperatingMode::Edit(_)) { + bail!( + "You are in conflict resolution mode. Finish with `but resolve finish` or cancel with `but resolve cancel` first." + ); + } + let Some(conflict_target) = target_conflicted_commit(ctx, commit)? else { + bail!("No conflicted commits found."); + }; + let commit_oid = conflict_target.commit_oid; + + // Split a trailing `:` off the target; everything else is the path. + let (path, hunk) = match target.rsplit_once(':') { + Some((path, number)) + if !number.is_empty() && number.bytes().all(|b| b.is_ascii_digit()) => + { + (path, Some(number.parse::()?)) + } + _ => (target, None), + }; + + let resolution = if ours { + HunkResolution::Ours + } else if theirs { + HunkResolution::Theirs + } else { + let content = match &file { + Some(file) => std::fs::read_to_string(file) + .with_context(|| format!("Failed to read {}", file.display()))?, + None => { + use std::io::{IsTerminal, Read}; + let mut stdin = std::io::stdin(); + if stdin.is_terminal() { + bail!( + "Provide the replacement content via --file or stdin, or take a side with --ours/--theirs." + ); + } + let mut content = String::new(); + stdin.read_to_string(&mut content)?; + if content.is_empty() { + bail!( + "stdin was empty. To intentionally delete the conflicted region, pass --file with an empty file." + ); + } + content + } + }; + HunkResolution::Content(content) + }; + + // Expand a bare path into explicit per-hunk specs; an explicit `:` + // target needs no lookup, the API validates it. + let hunks: Vec = match (hunk, &resolution) { + (Some(hunk), _) => vec![hunk], + (None, _) => { + let conflicts = but_api::resolve::commit_conflicts(ctx, commit_oid)?; + // Request paths are canonical; accept the common `./`-prefixed + // spelling the API would accept too. + let lookup = path.trim().trim_start_matches("./"); + let total_hunks = conflicts + .files + .iter() + .find(|conflicted| conflicted.path == lookup) + .map(|conflicted| conflicted.hunks.len()) + .with_context(|| format!("\"{path}\" is not a conflicted file of this commit"))?; + match &resolution { + HunkResolution::Ours | HunkResolution::Theirs => (1..=total_hunks).collect(), + HunkResolution::Content(_) if total_hunks == 1 => vec![1], + HunkResolution::Content(_) => bail!( + "\"{path}\" has {total_hunks} conflicts; content applies to one at a time, use \"{path}:\"." + ), + } + } + }; + let specs = hunks + .into_iter() + .map(|hunk| ResolutionSpec { + path: path.to_owned(), + hunk, + resolution: resolution.clone(), + }) + .collect(); + + let result = but_api::resolve::resolve_commit_conflict_hunks( + ctx, + commit_oid, + specs, + but_core::DryRun::No, + )?; + + if let Some(human_out) = out.for_human() { + let repo = ctx.repo.get()?; + writeln!( + human_out, + "{} {} {} {} {} {}", + t.success.paint(format!( + "✓ Resolved {} conflict{} in", + result.resolved, + if result.resolved == 1 { "" } else { "s" } + )), + t.attention.paint(path), + t.hint.paint("—"), + t.commit_id + .paint(shorten_object_id(&repo, result.commit_id)), + t.success.paint("→"), + t.commit_id + .paint(shorten_object_id(&repo, result.new_commit)), + )?; + if result.remaining.is_empty() { + writeln!( + human_out, + "{}", + t.success + .paint("All conflicts in this commit are resolved.") + )?; + drop(repo); + let more = oldest_conflicted_commit(ctx)?.is_some(); + if more { + writeln!( + human_out, + "{}", + t.attention + .paint("Other commits are still conflicted — run `but resolve conflicts`.") + )?; + } + } else { + for remaining in &result.remaining { + writeln!( + human_out, + "{}", + t.attention.paint(format!( + "{} conflict{} remaining in {} — run `but resolve conflicts` to see {}.", + remaining.hunks, + if remaining.hunks == 1 { "" } else { "s" }, + remaining.path, + if remaining.hunks == 1 { "it" } else { "them" }, + )) + )?; + } + } + writeln!( + human_out, + "{}", + t.hint + .paint("If this isn't right, run `but undo` to revert it.") + )?; + } + + if let Some(json_out) = out.for_json() { + json_out.write_value(serde_json::json!({ + "commit_id": result.commit_id.to_string(), + "new_commit_id": result.new_commit.to_string(), + "resolved": result.resolved, + "remaining": result.remaining, + }))?; + } + + Ok(()) +} + /// Resolve conflicts with the configured AI model: one commit when /// `commit_id_str` is given, otherwise every conflicted commit in the /// workspace, oldest first. diff --git a/crates/gitbutler-oplog/src/entry.rs b/crates/gitbutler-oplog/src/entry.rs index bbb3d90c8d8..e4c91560832 100644 --- a/crates/gitbutler-oplog/src/entry.rs +++ b/crates/gitbutler-oplog/src/entry.rs @@ -177,6 +177,7 @@ pub enum OperationKind { MoveCommitFile, FileChanges, EnterEditMode, + ResolveConflicts, ResolveConflictsAi, SyncWorkspace, CreateDependentBranch, @@ -238,6 +239,7 @@ impl OperationKind { | OperationKind::UpdateDependentBranchDescription | OperationKind::UpdateDependentBranchPrNumber => "UPDATE_BRANCH", OperationKind::SplitBranch => "SPLIT_BRANCH", + OperationKind::ResolveConflicts => "RESOLVE", OperationKind::ResolveConflictsAi => "AI_RESOLVE", OperationKind::StashIntoBranch | OperationKind::SetBaseBranch @@ -293,6 +295,7 @@ impl OperationKind { OperationKind::MoveCommitFile => "Moved file", OperationKind::FileChanges => "Updated file changes", OperationKind::EnterEditMode => "Entered edit mode", + OperationKind::ResolveConflicts => "Resolved conflicts", OperationKind::ResolveConflictsAi => "Resolved conflicts with AI", OperationKind::SyncWorkspace => "Synced workspace", OperationKind::CreateDependentBranch => "Created branch", @@ -350,6 +353,7 @@ impl OperationKind { OperationKind::MoveCommitFile => "MoveCommitFile", OperationKind::FileChanges => "FileChanges", OperationKind::EnterEditMode => "EnterEditMode", + OperationKind::ResolveConflicts => "ResolveConflicts", OperationKind::ResolveConflictsAi => "ResolveConflictsAi", OperationKind::SyncWorkspace => "SyncWorkspace", OperationKind::CreateDependentBranch => "CreateDependentBranch", @@ -407,6 +411,7 @@ impl OperationKind { "MoveCommitFile" => Self::MoveCommitFile, "FileChanges" => Self::FileChanges, "EnterEditMode" => Self::EnterEditMode, + "ResolveConflicts" => Self::ResolveConflicts, "ResolveConflictsAi" => Self::ResolveConflictsAi, "SyncWorkspace" => Self::SyncWorkspace, "CreateDependentBranch" => Self::CreateDependentBranch, diff --git a/crates/gitbutler-tauri/src/main.rs b/crates/gitbutler-tauri/src/main.rs index 0811c7750d3..38692c00759 100644 --- a/crates/gitbutler-tauri/src/main.rs +++ b/crates/gitbutler-tauri/src/main.rs @@ -526,6 +526,8 @@ fn main() -> anyhow::Result<()> { workspace::tauri_workspace_integrate_upstream::workspace_integrate_upstream, land::tauri_branch_land::branch_land, resolve::tauri_resolve_commit_conflicts_ai::resolve_commit_conflicts_ai, + resolve::tauri_commit_conflicts::commit_conflicts, + resolve::tauri_resolve_commit_conflict_hunks::resolve_commit_conflict_hunks, platform::tauri_build_type::build_type, ]) .menu(menu::build) diff --git a/packages/but-sdk/src/generated/graph/index.d.ts b/packages/but-sdk/src/generated/graph/index.d.ts index f4bc58cb7e2..9526178b94d 100644 --- a/packages/but-sdk/src/generated/graph/index.d.ts +++ b/packages/but-sdk/src/generated/graph/index.d.ts @@ -1158,6 +1158,14 @@ export type CommitAbsorption = { reason: AbsorptionReason; }; +/** JSON transport type for the conflicts of a conflicted commit. */ +export type CommitConflicts = { + /** The conflicted commit. */ + commitId: string; + /** The conflicted files, sorted by path. */ + files: Array; +}; + /** JSON transport type for creating a commit in the rebase graph. */ export type CommitCreateResult = { /** The new commit if one was created. */ @@ -1244,6 +1252,34 @@ export type ConflictEntryPresence = { ancestor: boolean; }; +/** + * One conflicted region of a file, with the content of each side and a few + * lines of surrounding context. + */ +export type ConflictHunk = { + /** Unconflicted lines directly before the conflict, clamped to the previous conflict. */ + contextBefore: string; + /** The content of the *ours* side, i.e. the new base the commit is rebased onto. */ + ours: string; + /** The content of the common ancestor, if the merge produced diff3-style markers. */ + base: string | null; + /** The content of the *theirs* side, i.e. the conflicted commit's own version. */ + theirs: string; + /** Unconflicted lines directly after the conflict, clamped to the next conflict. */ + contextAfter: string; +}; + +/** One conflicted file and its conflicts, in file order. */ +export type ConflictedFile = { + /** The repo-relative path of the file. */ + path: string; + /** + * The conflicts of the file; hunks are addressed by their 1-based + * position in this list. + */ + hunks: Array; +}; + /** A stack that conflicted while applying a branch. */ export type ConflictingStack = { /** The tip branch name of the stack. */ @@ -1876,6 +1912,30 @@ export type HunkLockTarget = { type: "unidentified"; }; +/** How to resolve a single conflict hunk. */ +export type HunkResolution = { + type: "ours"; +} | { + type: "theirs"; +} | { + type: "content"; + subject: string; +}; + +/** JSON transport type for the outcome of applying per-hunk resolutions. */ +export type HunkResolutionResult = { + /** The conflicted commit the resolutions were applied to. */ + commitId: string; + /** The rewritten commit. Still conflicted when `remaining` is non-empty. */ + newCommit: string; + /** How many conflicts were resolved. */ + resolved: number; + /** The conflicts that remain, per file. */ + remaining: Array; + /** Workspace state after the apply. */ + workspace: WorkspaceState; +}; + /** A way to indicate that a path in the index isn't suitable for committing and needs to be dealt with. */ export type IgnoredWorktreeChange = { /** The worktree-relative path to the change. */ @@ -2093,7 +2153,7 @@ export type OperatingMode = { subject: EditModeMetadata; }; -export type OperationKind = "CreateCommit" | "CreateBranch" | "StashIntoBranch" | "SetBaseBranch" | "MergeUpstream" | "UpdateWorkspaceBase" | "MoveHunk" | "UpdateBranchName" | "UpdateBranchNotes" | "ReorderBranches" | "UpdateBranchRemoteName" | "GenericBranchUpdate" | "DeleteBranch" | "ApplyBranch" | "DiscardLines" | "DiscardHunk" | "DiscardFile" | "DiscardChanges" | "Discard" | "AmendCommit" | "Absorb" | "AutoCommit" | "UndoCommit" | "DiscardCommit" | "UnapplyBranch" | "CherryPick" | "SquashCommit" | "UpdateCommitMessage" | "MoveCommit" | "MoveBranch" | "TearOffBranch" | "ReorderCommit" | "InsertBlankCommit" | "MoveCommitFile" | "FileChanges" | "EnterEditMode" | "ResolveConflictsAi" | "SyncWorkspace" | "CreateDependentBranch" | "RemoveDependentBranch" | "UpdateDependentBranchName" | "UpdateDependentBranchDescription" | "UpdateDependentBranchPrNumber" | "AutoHandleChangesBefore" | "AutoHandleChangesAfter" | "SplitBranch" | "CleanWorkspace" | "OnDemandSnapshot" | "Unknown" | "RestoreFromSnapshotViaUndo" | "RestoreFromSnapshotViaRedo" | "RestoreFromSnapshot"; +export type OperationKind = "CreateCommit" | "CreateBranch" | "StashIntoBranch" | "SetBaseBranch" | "MergeUpstream" | "UpdateWorkspaceBase" | "MoveHunk" | "UpdateBranchName" | "UpdateBranchNotes" | "ReorderBranches" | "UpdateBranchRemoteName" | "GenericBranchUpdate" | "DeleteBranch" | "ApplyBranch" | "DiscardLines" | "DiscardHunk" | "DiscardFile" | "DiscardChanges" | "Discard" | "AmendCommit" | "Absorb" | "AutoCommit" | "UndoCommit" | "DiscardCommit" | "UnapplyBranch" | "CherryPick" | "SquashCommit" | "UpdateCommitMessage" | "MoveCommit" | "MoveBranch" | "TearOffBranch" | "ReorderCommit" | "InsertBlankCommit" | "MoveCommitFile" | "FileChanges" | "EnterEditMode" | "ResolveConflicts" | "ResolveConflictsAi" | "SyncWorkspace" | "CreateDependentBranch" | "RemoveDependentBranch" | "UpdateDependentBranchName" | "UpdateDependentBranchDescription" | "UpdateDependentBranchPrNumber" | "AutoHandleChangesBefore" | "AutoHandleChangesAfter" | "SplitBranch" | "CleanWorkspace" | "OnDemandSnapshot" | "Unknown" | "RestoreFromSnapshotViaUndo" | "RestoreFromSnapshotViaRedo" | "RestoreFromSnapshot"; /** What kind of apply operation completed. */ export type OutcomeStatus = "alreadyApplied" | "applied" | "conflictAborted"; @@ -2260,6 +2320,14 @@ export type RelativeTo = { subject: Array; }; +/** Conflicts still unresolved in a file after an apply. */ +export type RemainingConflicts = { + /** The repo-relative path of the file. */ + path: string; + /** How many conflicts remain in it. */ + hunks: number; +}; + export type RemoteCommit = { id: string; description: string; @@ -2316,6 +2384,19 @@ export type ResolutionApproach = { type: "delete"; }; +/** + * One conflict to resolve, addressed by path and 1-based hunk index as + * returned by [`commit_conflicts()`]. + */ +export type ResolutionSpec = { + /** The repo-relative path of the conflicted file. */ + path: string; + /** The 1-based index of the conflict within the file. */ + hunk: number; + /** How to resolve it. */ + resolution: HunkResolution; +}; + /** How one conflicted file was resolved, for display to the user. */ export type ResolvedFile = { /** The repo-relative path of the file. */ diff --git a/packages/but-sdk/src/generated/linear/index.d.ts b/packages/but-sdk/src/generated/linear/index.d.ts index f7ba4f7db37..99cd93127cf 100644 --- a/packages/but-sdk/src/generated/linear/index.d.ts +++ b/packages/but-sdk/src/generated/linear/index.d.ts @@ -1158,6 +1158,14 @@ export type CommitAbsorption = { reason: AbsorptionReason; }; +/** JSON transport type for the conflicts of a conflicted commit. */ +export type CommitConflicts = { + /** The conflicted commit. */ + commitId: string; + /** The conflicted files, sorted by path. */ + files: Array; +}; + /** JSON transport type for creating a commit in the rebase graph. */ export type CommitCreateResult = { /** The new commit if one was created. */ @@ -1244,6 +1252,34 @@ export type ConflictEntryPresence = { ancestor: boolean; }; +/** + * One conflicted region of a file, with the content of each side and a few + * lines of surrounding context. + */ +export type ConflictHunk = { + /** Unconflicted lines directly before the conflict, clamped to the previous conflict. */ + contextBefore: string; + /** The content of the *ours* side, i.e. the new base the commit is rebased onto. */ + ours: string; + /** The content of the common ancestor, if the merge produced diff3-style markers. */ + base: string | null; + /** The content of the *theirs* side, i.e. the conflicted commit's own version. */ + theirs: string; + /** Unconflicted lines directly after the conflict, clamped to the next conflict. */ + contextAfter: string; +}; + +/** One conflicted file and its conflicts, in file order. */ +export type ConflictedFile = { + /** The repo-relative path of the file. */ + path: string; + /** + * The conflicts of the file; hunks are addressed by their 1-based + * position in this list. + */ + hunks: Array; +}; + /** A stack that conflicted while applying a branch. */ export type ConflictingStack = { /** The tip branch name of the stack. */ @@ -1876,6 +1912,30 @@ export type HunkLockTarget = { type: "unidentified"; }; +/** How to resolve a single conflict hunk. */ +export type HunkResolution = { + type: "ours"; +} | { + type: "theirs"; +} | { + type: "content"; + subject: string; +}; + +/** JSON transport type for the outcome of applying per-hunk resolutions. */ +export type HunkResolutionResult = { + /** The conflicted commit the resolutions were applied to. */ + commitId: string; + /** The rewritten commit. Still conflicted when `remaining` is non-empty. */ + newCommit: string; + /** How many conflicts were resolved. */ + resolved: number; + /** The conflicts that remain, per file. */ + remaining: Array; + /** Workspace state after the apply. */ + workspace: WorkspaceState; +}; + /** A way to indicate that a path in the index isn't suitable for committing and needs to be dealt with. */ export type IgnoredWorktreeChange = { /** The worktree-relative path to the change. */ @@ -2093,7 +2153,7 @@ export type OperatingMode = { subject: EditModeMetadata; }; -export type OperationKind = "CreateCommit" | "CreateBranch" | "StashIntoBranch" | "SetBaseBranch" | "MergeUpstream" | "UpdateWorkspaceBase" | "MoveHunk" | "UpdateBranchName" | "UpdateBranchNotes" | "ReorderBranches" | "UpdateBranchRemoteName" | "GenericBranchUpdate" | "DeleteBranch" | "ApplyBranch" | "DiscardLines" | "DiscardHunk" | "DiscardFile" | "DiscardChanges" | "Discard" | "AmendCommit" | "Absorb" | "AutoCommit" | "UndoCommit" | "DiscardCommit" | "UnapplyBranch" | "CherryPick" | "SquashCommit" | "UpdateCommitMessage" | "MoveCommit" | "MoveBranch" | "TearOffBranch" | "ReorderCommit" | "InsertBlankCommit" | "MoveCommitFile" | "FileChanges" | "EnterEditMode" | "ResolveConflictsAi" | "SyncWorkspace" | "CreateDependentBranch" | "RemoveDependentBranch" | "UpdateDependentBranchName" | "UpdateDependentBranchDescription" | "UpdateDependentBranchPrNumber" | "AutoHandleChangesBefore" | "AutoHandleChangesAfter" | "SplitBranch" | "CleanWorkspace" | "OnDemandSnapshot" | "Unknown" | "RestoreFromSnapshotViaUndo" | "RestoreFromSnapshotViaRedo" | "RestoreFromSnapshot"; +export type OperationKind = "CreateCommit" | "CreateBranch" | "StashIntoBranch" | "SetBaseBranch" | "MergeUpstream" | "UpdateWorkspaceBase" | "MoveHunk" | "UpdateBranchName" | "UpdateBranchNotes" | "ReorderBranches" | "UpdateBranchRemoteName" | "GenericBranchUpdate" | "DeleteBranch" | "ApplyBranch" | "DiscardLines" | "DiscardHunk" | "DiscardFile" | "DiscardChanges" | "Discard" | "AmendCommit" | "Absorb" | "AutoCommit" | "UndoCommit" | "DiscardCommit" | "UnapplyBranch" | "CherryPick" | "SquashCommit" | "UpdateCommitMessage" | "MoveCommit" | "MoveBranch" | "TearOffBranch" | "ReorderCommit" | "InsertBlankCommit" | "MoveCommitFile" | "FileChanges" | "EnterEditMode" | "ResolveConflicts" | "ResolveConflictsAi" | "SyncWorkspace" | "CreateDependentBranch" | "RemoveDependentBranch" | "UpdateDependentBranchName" | "UpdateDependentBranchDescription" | "UpdateDependentBranchPrNumber" | "AutoHandleChangesBefore" | "AutoHandleChangesAfter" | "SplitBranch" | "CleanWorkspace" | "OnDemandSnapshot" | "Unknown" | "RestoreFromSnapshotViaUndo" | "RestoreFromSnapshotViaRedo" | "RestoreFromSnapshot"; /** What kind of apply operation completed. */ export type OutcomeStatus = "alreadyApplied" | "applied" | "conflictAborted"; @@ -2260,6 +2320,14 @@ export type RelativeTo = { subject: Array; }; +/** Conflicts still unresolved in a file after an apply. */ +export type RemainingConflicts = { + /** The repo-relative path of the file. */ + path: string; + /** How many conflicts remain in it. */ + hunks: number; +}; + export type RemoteCommit = { id: string; description: string; @@ -2316,6 +2384,19 @@ export type ResolutionApproach = { type: "delete"; }; +/** + * One conflict to resolve, addressed by path and 1-based hunk index as + * returned by [`commit_conflicts()`]. + */ +export type ResolutionSpec = { + /** The repo-relative path of the conflicted file. */ + path: string; + /** The 1-based index of the conflict within the file. */ + hunk: number; + /** How to resolve it. */ + resolution: HunkResolution; +}; + /** How one conflicted file was resolved, for display to the user. */ export type ResolvedFile = { /** The repo-relative path of the file. */ diff --git a/packages/ui/src/lib/utils/testIds.ts b/packages/ui/src/lib/utils/testIds.ts index 64b9e9c0854..179419591e0 100644 --- a/packages/ui/src/lib/utils/testIds.ts +++ b/packages/ui/src/lib/utils/testIds.ts @@ -47,6 +47,7 @@ export enum TestId { CommitRowContextMenu_EditMessageMenuButton = "commit-row-context-menu-edit-message-menu-btn", CommitRowContextMenu_EditCommit = "commit-row-context-menu-edit-commit", CommitRowContextMenu_ResolveConflictsAi = "commit-row-context-menu-resolve-conflicts-ai", + CommitRowContextMenu_ShowConflicts = "commit-row-context-menu-show-conflicts", CommitRowContextMenu_SquashSelected = "commit-row-context-menu-squash-selected", CommitRowContextMenu_UncommitSelected = "commit-row-context-menu-uncommit-selected", NewCommitView = "new-commit-view", From e909431cd670a750b0f60d912cbd27d6b71de8a6 Mon Sep 17 00:00:00 2001 From: Kiril Videlov Date: Sat, 4 Jul 2026 11:28:09 +0200 Subject: [PATCH 2/3] Show conflicts inline in the commit diff pane MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clicking a conflicted commit now renders a real diff for every conflicted file. commit_conflicts returns a synthetic TreeChange per file diffing the current base's version (ours) against the commit's own (theirs) — regular commit diffs are computed against the auto-resolution, which hides exactly the conflicted content. The diff view marks such files with a banner and a Show/Hide toggle that interleaves ours/base/theirs conflict cards at the right position via a new ConflictHunk.line anchor. The file list no longer filters conflicted files, and clicking an unrepresented one jumps to its entry in the diff pane. The right-click 'Show conflicts' modal is superseded and removed. --- .../commit/CommitContextMenu.svelte | 16 --- .../commit/ConflictHunksModal.svelte | 104 ------------------ .../components/diff/ConflictHunkCard.svelte | 90 +++++++++++++++ .../src/components/diff/MultiDiffView.svelte | 40 +++++-- .../src/components/diff/SelectionView.svelte | 16 +++ .../components/diff/UnifiedDiffView.svelte | 94 +++++++++++++++- .../components/files/ChangedFilesPanel.svelte | 3 + .../components/files/FileListConflicts.svelte | 19 +++- .../components/views/BranchCommitList.svelte | 14 ++- .../src/components/views/StackDetails.svelte | 9 ++ .../src/lib/stacks/stackController.svelte.ts | 13 ++- crates/but-api/src/resolve/apply.rs | 1 + crates/but-api/src/resolve/context.rs | 71 +++++++----- crates/but-api/src/resolve/mod.rs | 62 +++++++++-- crates/but-api/tests/api/resolve_hunks.rs | 18 ++- .../but-sdk/src/generated/graph/index.d.ts | 14 +++ .../but-sdk/src/generated/linear/index.d.ts | 14 +++ packages/ui/src/lib/utils/testIds.ts | 1 - 18 files changed, 426 insertions(+), 173 deletions(-) delete mode 100644 apps/desktop/src/components/commit/ConflictHunksModal.svelte create mode 100644 apps/desktop/src/components/diff/ConflictHunkCard.svelte diff --git a/apps/desktop/src/components/commit/CommitContextMenu.svelte b/apps/desktop/src/components/commit/CommitContextMenu.svelte index 64186501a0b..8bd85af3bcf 100644 --- a/apps/desktop/src/components/commit/CommitContextMenu.svelte +++ b/apps/desktop/src/components/commit/CommitContextMenu.svelte @@ -53,7 +53,6 @@ - - - {#if conflictsQuery} - - {#snippet children(conflicts)} -
- {#each conflicts.files as file (file.path)} -
-

{file.path}

- {#each file.hunks as hunk, index} -
-
- Conflict {index + 1} of {file.hunks.length} -
-
ours — the new base
- 
{hunk.ours}
- {#if hunk.base !== null} -
base — common ancestor
- 
{hunk.base}
- {/if} -
theirs — this commit
- 
{hunk.theirs}
-
- {/each} -
- {/each} -
- {/snippet} - - {/if} - - {#snippet controls(close)} - - {/snippet} - - - diff --git a/apps/desktop/src/components/diff/ConflictHunkCard.svelte b/apps/desktop/src/components/diff/ConflictHunkCard.svelte new file mode 100644 index 00000000000..6051ff838f6 --- /dev/null +++ b/apps/desktop/src/components/diff/ConflictHunkCard.svelte @@ -0,0 +1,90 @@ + + +
+
+ + + Unresolved conflict{total > 1 ? ` ${index + 1} of ${total}` : ""} + +
+
+
Current base
+ 
{hunk.ours}
+
+ {#if hunk.base !== null} +
+
Common ancestor
+ 
{hunk.base}
+
+ {/if} +
+
This commit
+ 
{hunk.theirs}
+
+
+ + diff --git a/apps/desktop/src/components/diff/MultiDiffView.svelte b/apps/desktop/src/components/diff/MultiDiffView.svelte index 7e9d7e61937..2d526bdbb3b 100644 --- a/apps/desktop/src/components/diff/MultiDiffView.svelte +++ b/apps/desktop/src/components/diff/MultiDiffView.svelte @@ -24,7 +24,7 @@ import { inject } from "@gitbutler/core/context"; import { Button, FileViewHeader, HunkDiffSkeleton, VirtualList } from "@gitbutler/ui"; import { untrack } from "svelte"; - import type { TreeChange } from "@gitbutler/but-sdk"; + import type { ConflictedFile, TreeChange } from "@gitbutler/but-sdk"; type Props = { projectId: string; @@ -36,6 +36,8 @@ showRoundedEdges?: boolean; startIndex?: number; selectionId: SelectionId; + /// The selected commit's unresolved conflicts, shown inline per file. + conflicts?: ConflictedFile[]; onclose?: () => void; onVisibleChange?: (change: { start: number; end: number } | undefined) => void; }; @@ -50,10 +52,21 @@ showRoundedEdges = true, startIndex, selectionId, + conflicts, onclose, onVisibleChange, }: Props = $props(); + // Conflicted files whose diff is hidden by the auto-resolution get their + // synthetic base-vs-commit change appended, after `changes` so the indices + // callers use for jumping and selection stay stable. + const items: TreeChange[] = $derived([ + ...changes, + ...(conflicts ?? []) + .filter((file) => !changes.some((change) => change.path === file.path)) + .map((file) => file.change), + ]); + const diffService = inject(DIFF_SERVICE); const idSelection = inject(FILE_SELECTION_MANAGER); const uiState = inject(UI_STATE); @@ -87,6 +100,14 @@ } } + // Jump by path; this component owns the true render order of `items` + // (changes followed by appended conflict-only entries), so callers don't + // have to re-derive it. + export function jumpToPath(path: string) { + const index = items.findIndex((item) => item.path === path); + if (index >= 0) jumpToIndex(index); + } + export function openFloatingDiff() { floatingDiffInitialIndex = highlightedIndex ?? startIndex ?? 0; floatingDiffOpen = true; @@ -110,6 +131,7 @@ {@const diffQuery = diffService.getDiff(projectId, change)} {@const diffData = diffQuery.response} {@const isExecutable = isExecutableStatus(change.status)} + {@const conflictHunks = conflicts?.find((file) => file.path === change.path)?.hunks} {@const patchData = diffData?.type === "Patch" ? diffData.subject : null} {@const isCollapsed = diffExpandedState.get(change.path) ?? false} {/snippet} @@ -198,7 +222,7 @@ {/if} - {#if changes && changes.length > 0} + {#if items.length > 0} {#if !allInOneDiff} {@const index = highlightedIndex ?? startIndex ?? 0} - {@const change = changes[index]} + {@const change = items[index]} {#if change}
{@render changeItem(change, index)} @@ -221,7 +245,7 @@ bind:this={virtualList} {startIndex} grow - items={changes} + {items} defaultHeight={173} visibility="scroll" renderDistance={100} @@ -231,7 +255,7 @@ const activeIndex = scrollLock.resolve(range); highlightedIndex = activeIndex; - const activeChange = changes[activeIndex]; + const activeChange = items[activeIndex]; const selectionSize = idSelection.collectionSize(selectionId); const shouldFollowScrollSelection = selectionSize <= 1; if ( @@ -244,10 +268,10 @@ } onVisibleChange?.(range); }} - getId={(change) => change.path} + getId={(item) => item.path} > - {#snippet template(change, index)} - {@render changeItem(change, index, true)} + {#snippet template(item, index)} + {@render changeItem(item, index, true)} {/snippet} {/if} diff --git a/apps/desktop/src/components/diff/SelectionView.svelte b/apps/desktop/src/components/diff/SelectionView.svelte index de3628de1e5..d6e833d1efd 100644 --- a/apps/desktop/src/components/diff/SelectionView.svelte +++ b/apps/desktop/src/components/diff/SelectionView.svelte @@ -7,6 +7,7 @@ import { DIFF_SERVICE } from "$lib/hunks/diffService.svelte"; import { FILE_SELECTION_MANAGER } from "$lib/selection/fileSelectionManager.svelte"; import { readKey, type SelectionId } from "$lib/selection/key"; + import { STACK_SERVICE } from "$lib/stacks/stackService.svelte"; import { inject } from "@gitbutler/core/context"; type Props = { @@ -33,6 +34,7 @@ const idSelection = inject(FILE_SELECTION_MANAGER); const diffService = inject(DIFF_SERVICE); + const stackService = inject(STACK_SERVICE); const selection = $derived(selectionId ? idSelection.valuesReactive(selectionId) : undefined); const lastAdded = $derived(selectionId ? idSelection.getById(selectionId).lastAdded : undefined); @@ -49,6 +51,19 @@ ); const selectable = $derived(selectionId?.type === "worktree"); + + // For commit selections, offer the selected file's unresolved conflicts + // inline in the diff. commit_conflicts returns no files for an unconflicted + // commit, so it needs no gate. + const selectedCommitId = $derived( + selectedFile?.type === "commit" ? selectedFile.commitId : undefined, + ); + const conflictsQuery = $derived( + selectedCommitId ? stackService.commitConflicts(projectId, selectedCommitId) : undefined, + ); + const selectedFileConflicts = $derived( + conflictsQuery?.response?.files.find((file) => file.path === selectedFile?.path)?.hunks, + );
@@ -86,6 +101,7 @@ {diff} {selectable} selectionId={selectedFile} + conflictHunks={selectedFileConflicts} topPadding={diffOnly} />
diff --git a/apps/desktop/src/components/diff/UnifiedDiffView.svelte b/apps/desktop/src/components/diff/UnifiedDiffView.svelte index 0182bfdd9d4..c7b27a41f2d 100644 --- a/apps/desktop/src/components/diff/UnifiedDiffView.svelte +++ b/apps/desktop/src/components/diff/UnifiedDiffView.svelte @@ -1,4 +1,5 @@ +
@@ -32,6 +111,73 @@
This commit
{hunk.theirs}
+ {#if commitId && path} + {#if editing} +
+
+ Edit the merged result that replaces this conflict. Leaving it empty deletes the region. +
+ + +
+ + +
+
+ {:else} +
+ + + + {#if $aiGenEnabled} + + {/if} +
+ {/if} + {/if}
diff --git a/apps/desktop/src/components/diff/MultiDiffView.svelte b/apps/desktop/src/components/diff/MultiDiffView.svelte index 2d526bdbb3b..305f10a7953 100644 --- a/apps/desktop/src/components/diff/MultiDiffView.svelte +++ b/apps/desktop/src/components/diff/MultiDiffView.svelte @@ -233,7 +233,7 @@ }} /> {#if !allInOneDiff} - {@const index = highlightedIndex ?? startIndex ?? 0} + {@const index = Math.min(highlightedIndex ?? startIndex ?? 0, items.length - 1)} {@const change = items[index]} {#if change}
@@ -284,7 +284,7 @@ +{#snippet conflictCards(bucket: { hunk: ConflictHunk; index: number }[])} + {#each bucket as conflict (conflict.index)} + + {/each} +{/snippet} + {#if fileDependenciesQuery} {:else} @@ -295,6 +309,9 @@
{/if} {#if linesModified > LARGE_DIFF_THRESHOLD && !showAnyways} + {#if showConflicts && conflictHunks} + {@render conflictCards(conflictHunks.map((hunk, index) => ({ hunk, index })))} + {/if} { showAnyways = true; @@ -306,13 +323,7 @@ {@const [_, lineLocks] = getLineLocks(hunk, fileDependencies?.dependencies ?? [])} {@const hunkId = generateHunkId(change.path, hunkIndex)} {@const reactions = fileReactions[hunkKey(hunk)] ?? []} - {#each conflictBuckets[hunkIndex] ?? [] as conflict (conflict.index)} - - {/each} + {@render conflictCards(conflictBuckets[hunkIndex] ?? [])}
{:else} {#if showConflicts} - {#each conflictBuckets[0] ?? [] as conflict (conflict.index)} - - {/each} + {@render conflictCards(conflictBuckets[0] ?? [])} {:else if diff.subject.hunks.length === 0}
@@ -449,13 +454,7 @@ {/if} {/each} {#if filteredHunks.length > 0 && renderedHunkCount >= filteredHunks.length} - {#each conflictBuckets[filteredHunks.length] ?? [] as conflict (conflict.index)} - - {/each} + {@render conflictCards(conflictBuckets[filteredHunks.length] ?? [])} {/if} {/if} {:else if diff.type === "TooLarge"} diff --git a/apps/desktop/src/lib/stacks/stackController.svelte.ts b/apps/desktop/src/lib/stacks/stackController.svelte.ts index 23de78babcb..cefe73702d6 100644 --- a/apps/desktop/src/lib/stacks/stackController.svelte.ts +++ b/apps/desktop/src/lib/stacks/stackController.svelte.ts @@ -38,6 +38,11 @@ export function getStackContext(): StackController { return ctx; } +/** Like [getStackContext], for components that also render outside a StackView. */ +export function maybeGetStackContext(): StackController | undefined { + return getContext(STACK_CTX); +} + export class StackController { private uiState; private fileSelection: FileSelectionManager; diff --git a/apps/desktop/src/lib/stacks/stackEndpoints.ts b/apps/desktop/src/lib/stacks/stackEndpoints.ts index 8e13ce68976..dee19dca159 100644 --- a/apps/desktop/src/lib/stacks/stackEndpoints.ts +++ b/apps/desktop/src/lib/stacks/stackEndpoints.ts @@ -28,6 +28,8 @@ import type { AiResolutionResult, BranchLandResult, CommitConflicts, + HunkResolutionResult, + ResolutionSpec, CommitAbsorption, BranchDetails, BranchReference, @@ -486,6 +488,26 @@ export function buildStackEndpoints(build: BackendEndpointBuilder) { ...(stackId ? [invalidatesItem(ReduxTag.StackDetails, stackId)] : []), ], }), + resolveCommitConflictHunks: build.mutation< + HunkResolutionResult, + { projectId: string; stackId?: string; commitId: string; specs: ResolutionSpec[] } + >({ + extraOptions: { + command: "resolve_commit_conflict_hunks", + actionName: "Resolve Conflict", + }, + query: ({ projectId, commitId, specs }) => ({ + projectId, + commitId, + specs, + }), + invalidatesTags: (_result, _error, { stackId }) => [ + invalidatesList(ReduxTag.HeadSha), + invalidatesList(ReduxTag.BranchChanges), + invalidatesList(ReduxTag.WorktreeChanges), + ...(stackId ? [invalidatesItem(ReduxTag.StackDetails, stackId)] : []), + ], + }), newBranch: build.mutation< void, { projectId: string; stackId: string; request: { targetPatch?: string; name: string } } diff --git a/apps/desktop/src/lib/stacks/stackService.svelte.ts b/apps/desktop/src/lib/stacks/stackService.svelte.ts index 094fe7b03c8..7af2e9b5e19 100644 --- a/apps/desktop/src/lib/stacks/stackService.svelte.ts +++ b/apps/desktop/src/lib/stacks/stackService.svelte.ts @@ -554,6 +554,10 @@ export class StackService { return this.backendApi.endpoints.resolveCommitConflictsAi.useMutation(); } + get resolveCommitConflictHunks() { + return this.backendApi.endpoints.resolveCommitConflictHunks.useMutation(); + } + commitConflicts(projectId: string, commitId: string) { return this.backendApi.endpoints.commitConflicts.useQuery({ projectId, commitId }); } diff --git a/crates/but-api/src/resolve/apply.rs b/crates/but-api/src/resolve/apply.rs index 31fe2d6bda4..f4b1d1ac793 100644 --- a/crates/but-api/src/resolve/apply.rs +++ b/crates/but-api/src/resolve/apply.rs @@ -1,11 +1,13 @@ //! Turn per-hunk resolutions into a rewritten commit. //! //! Resolutions are applied by *narrowing the conflict*: each resolved hunk's -//! content is written into all three side blobs (ours/base/theirs), while -//! unresolved hunks keep each side's original lines. Re-merging the synthesized -//! trees then conflicts exactly at the unresolved hunks — so a partial -//! resolution yields a conflicted commit with fewer conflicts, and a full -//! resolution falls out of the very same path as a clean merge. +//! content is written into the ours and theirs side blobs (the base is left +//! untouched), while unresolved hunks keep each side's original lines. +//! Re-merging the synthesized trees then conflicts exactly at the unresolved +//! hunks — so a partial resolution yields a conflicted commit with fewer +//! conflicts, and a full resolution falls out of the very same path as a clean +//! merge. Leaving the base unmodified means a later re-pick onto changed +//! parents replays the resolution instead of silently dropping it. use std::collections::BTreeMap; @@ -26,6 +28,9 @@ use crate::WorkspaceState; pub(crate) struct AppliedResolution { /// The rewritten commit's final id after the rebase. pub new_commit: gix::ObjectId, + /// Whether the fully resolved commit ended up with the same tree as its + /// parent, i.e. the resolutions dropped all of its changes. + pub commit_emptied: bool, /// The conflicts that remain per file; empty when fully resolved. pub remaining: Vec, /// Workspace state after the apply. @@ -57,21 +62,8 @@ pub(crate) fn validate_specs( let mut picks: PicksPerFile = vec![BTreeMap::new(); request.files.len()]; for spec in specs { - let &file_index = files_by_path - .get(&normalize_path(&spec.path)) - .with_context(|| { - format!("\"{}\" is not a conflicted file of this commit", spec.path) - })?; + let (file_index, hunk_index) = locate_hunk(request, &files_by_path, &spec.path, spec.hunk)?; let file = &request.files[file_index]; - if spec.hunk == 0 || spec.hunk > file.hunks.len() { - bail!( - "\"{}\" has conflicts 1..{}, but conflict {} was addressed", - file.path, - file.hunks.len(), - spec.hunk - ); - } - let hunk_index = spec.hunk - 1; let pick = match &spec.resolution { HunkResolution::Ours => HunkPick::Ours, HunkResolution::Theirs => HunkPick::Theirs, @@ -79,6 +71,8 @@ pub(crate) fn validate_specs( ensure_no_markers(content, &file.path)?; HunkPick::Content(content.clone()) } + // Replaced with `Content` by `resolve_ai_specs()` before validation. + HunkResolution::Ai => bail!("AI resolutions must be materialized before validation"), }; if picks[file_index].insert(hunk_index, pick).is_some() { bail!( @@ -92,6 +86,30 @@ pub(crate) fn validate_specs( Ok(picks) } +/// Resolve a `(path, 1-based hunk)` address against the request, returning +/// the file index and 0-based hunk index. +pub(crate) fn locate_hunk( + request: &ResolutionRequest, + files_by_path: &BTreeMap, + path: &str, + hunk: usize, +) -> anyhow::Result<(usize, usize)> { + let &file_index = files_by_path + .get(&normalize_path(path)) + .with_context(|| format!("\"{path}\" is not a conflicted file of this commit"))?; + let file = &request.files[file_index]; + if hunk == 0 || hunk > file.hunks.len() { + bail!( + "\"{}\" has {} conflict{}, but conflict {} was addressed", + file.path, + file.hunks.len(), + if file.hunks.len() == 1 { "" } else { "s" }, + hunk + ); + } + Ok((file_index, hunk - 1)) +} + /// Map normalized request paths to file indices, rejecting collisions. pub(crate) fn index_files_by_path( request: &ResolutionRequest, @@ -125,8 +143,11 @@ pub(crate) fn ensure_no_markers(content: &str, path: &str) -> anyhow::Result<()> /// Normalize a path for comparison: trim, backslashes to slashes, strip a /// leading `./`, collapse duplicate slashes. Applied to both request and -/// caller-provided paths. -pub(crate) fn normalize_path(path: &str) -> String { +/// caller-provided paths, so callers matching against [`ConflictedFile`] paths +/// should normalize with this too. +/// +/// [`ConflictedFile`]: super::ConflictedFile +pub fn normalize_path(path: &str) -> String { let mut normalized = path.trim().replace('\\', "/"); while normalized.contains("//") { normalized = normalized.replace("//", "/"); @@ -140,7 +161,6 @@ pub(crate) fn normalize_path(path: &str) -> String { #[derive(Debug, Clone, Copy)] enum SideKind { Ours, - Base, Theirs, } @@ -227,12 +247,6 @@ fn synthesize_side( let side_range = match side { SideKind::Ours => block.ours.clone(), SideKind::Theirs => block.theirs.clone(), - SideKind::Base => block.base.clone().with_context(|| { - format!( - "A conflict in \"{}\" carries no common-ancestor section and cannot be narrowed. Resolve this commit manually instead.", - file.path - ) - })?, }; for raw in &raw_lines[side_range] { result.push_str(raw); @@ -268,10 +282,13 @@ pub(crate) fn apply( let mut meta = ctx.meta()?; let (repo, mut ws, _) = ctx.workspace_mut_and_db_with_perm(perm)?; - // Narrow the sides: every resolved hunk's content goes into all three - // trees, so the re-merge below agrees there and conflicts only at what - // remains unresolved. - let mut base_edit = repo.edit_tree(request.base_tree_id)?; + // Narrow the sides: every resolved hunk's content goes into both side + // trees, which the re-merge below sees as an identical change against the + // untouched base and resolves cleanly to the picked content. The base + // must stay untouched: with base==theirs a resolved region would read as + // "this commit does not change it", so a later re-pick of a + // still-conflicted commit onto changed parents would silently drop the + // resolution in favor of the new base instead of replaying it. let mut ours_edit = repo.edit_tree(request.ours_tree_id)?; let mut theirs_edit = repo.edit_tree(request.theirs_tree_id)?; for (file, picks) in request.files.iter().zip(picks_per_file) { @@ -279,7 +296,6 @@ pub(crate) fn apply( continue; } for (tree, side) in [ - (&mut base_edit, SideKind::Base), (&mut ours_edit, SideKind::Ours), (&mut theirs_edit, SideKind::Theirs), ] { @@ -288,7 +304,7 @@ pub(crate) fn apply( tree.upsert(file.rela_path.as_bstr(), file.entry_kind, blob_id)?; } } - let base_tree_id = base_edit.write()?.detach(); + let base_tree_id = request.base_tree_id; let ours_tree_id = ours_edit.write()?.detach(); let theirs_tree_id = theirs_edit.write()?.detach(); @@ -328,6 +344,18 @@ pub(crate) fn apply( let mut editor = Editor::create(&mut ws, &mut meta, &repo)?; let (target_selector, mut commit) = editor.find_selectable_commit(request.commit_id)?; + // Fully resolving in favor of the base can leave the commit with no + // changes of its own — legitimate, but worth telling the user about. + // The parent's content is its auto-resolution when the parent is itself + // still conflicted, never its raw (wrapper) tree. + use gix::prelude::ObjectIdExt as _; + let commit_emptied = !unresolved + && match *commit.parents.as_slice() { + [parent] => but_core::Commit::from_id(parent.attach(&repo)) + .and_then(|parent| parent.tree_id_or_auto_resolution()) + .is_ok_and(|parent_tree| parent_tree.detach() == merged_tree_id), + _ => false, + }; if unresolved { // Still conflicted: same commit shape the rebase engine writes, with // the narrowed trees. Adding message markers is idempotent, and covers @@ -370,6 +398,7 @@ pub(crate) fn apply( Ok(AppliedResolution { new_commit, + commit_emptied, remaining, workspace, }) @@ -424,7 +453,7 @@ mod tests { let file = two_hunks(); let all = picks(&[(0, "merged one"), (1, "merged two")]); let expected = "start\nmerged one\nmiddle\nmerged two\nend\n"; - for side in [SideKind::Ours, SideKind::Base, SideKind::Theirs] { + for side in [SideKind::Ours, SideKind::Theirs] { assert_eq!(synthesize_side(&file, &all, side).unwrap(), expected); } } @@ -437,10 +466,6 @@ mod tests { synthesize_side(&file, &first_only, SideKind::Ours).unwrap(), "start\nmerged one\nmiddle\nours two\nend\n" ); - assert_eq!( - synthesize_side(&file, &first_only, SideKind::Base).unwrap(), - "start\nmerged one\nmiddle\nbase two\nend\n" - ); assert_eq!( synthesize_side(&file, &first_only, SideKind::Theirs).unwrap(), "start\nmerged one\nmiddle\ntheirs two\nend\n" @@ -525,6 +550,14 @@ mod tests { assert!(matches!(&picks[0][&1], HunkPick::Content(content) if content == "mixed")); } + #[test] + fn normalize_path_canonicalizes_separators_and_prefix() { + assert_eq!(normalize_path(" ./a/b.txt "), "a/b.txt"); + assert_eq!(normalize_path("a\\b\\c.txt"), "a/b/c.txt"); + assert_eq!(normalize_path("a//b///c.txt"), "a/b/c.txt"); + assert_eq!(normalize_path("a/b.txt"), "a/b.txt"); + } + /// A side pick must reproduce the picked side byte-for-byte — including a /// trailing blank line and the side's own line terminators. #[test] @@ -533,7 +566,7 @@ mod tests { "start\n{OURS}\nfoo\n\n{BASE}\nbase\n=======\ntheirs one\r\ntheirs two\n{THEIRS}\nend\n" )); let ours: BTreeMap = [(0, HunkPick::Ours)].into_iter().collect(); - for side in [SideKind::Ours, SideKind::Base, SideKind::Theirs] { + for side in [SideKind::Ours, SideKind::Theirs] { assert_eq!( synthesize_side(&file, &ours, side).unwrap(), "start\nfoo\n\nend\n", @@ -561,12 +594,12 @@ mod tests { assert!( err(&[spec("file.txt", 3, HunkResolution::Ours)]) .to_string() - .contains("has conflicts 1..2") + .contains("has 2 conflicts, but conflict 3 was addressed") ); assert!( err(&[spec("file.txt", 0, HunkResolution::Ours)]) .to_string() - .contains("has conflicts 1..2") + .contains("has 2 conflicts, but conflict 0 was addressed") ); assert!( err(&[ diff --git a/crates/but-api/src/resolve/mod.rs b/crates/but-api/src/resolve/mod.rs index 31aa5e1f6b3..126f6090450 100644 --- a/crates/but-api/src/resolve/mod.rs +++ b/crates/but-api/src/resolve/mod.rs @@ -3,13 +3,13 @@ //! GitButler keeps conflicts as first-class committed state: a conflicted //! commit carries its merge inputs as trees. This module re-merges those trees //! in memory to expose the conflicts as per-file hunks -//! ([`commit_conflicts()`]), and applies per-hunk resolutions — a side pick or +//! (`commit_conflicts()`), and applies per-hunk resolutions — a side pick or //! custom content, for some or all hunks — by narrowing the conflict and //! rewriting the commit, rebasing its descendants -//! ([`resolve_commit_conflict_hunks()`]). A partial resolution leaves the +//! (`resolve_commit_conflict_hunks()`). A partial resolution leaves the //! commit conflicted with fewer conflicts, so callers can work incrementally. //! -//! [`resolve_commit_conflicts_ai()`] is a client of the same core: it sends +//! `resolve_commit_conflicts_ai()` is a client of the same core: it sends //! the hunks to the configured LLM as a single-shot, no-tools request and //! applies the returned full-coverage resolutions. In every case the workspace //! never enters edit mode, the exclusive worktree lock is held only for the @@ -33,6 +33,7 @@ mod apply; mod context; mod prompt; +pub use apply::normalize_path; pub use context::{ConflictHunk, FileConflict, ResolutionRequest}; pub use prompt::{FileResolution, HunkContent, ResolutionResponse, SYSTEM_PROMPT}; @@ -71,7 +72,7 @@ but_schemars::register_sdk_type!(ConflictedFile); /// /// Hunks are identified by `(path, 1-based index)`; the extraction is /// deterministic, so the same commit id always yields the same hunks and -/// [`resolve_commit_conflict_hunks()`] can be called with indices from this +/// `resolve_commit_conflict_hunks()` can be called with indices from this /// result. Fails for commits whose conflicts have no hunk representation /// (deletions/renames, binaries, oversized files, marker-like content) — those /// need manual resolution in edit mode. @@ -81,8 +82,19 @@ pub fn commit_conflicts( ctx: &but_ctx::Context, commit_id: gix::ObjectId, ) -> anyhow::Result { + use gix::prelude::ObjectIdExt as _; + let _guard = ctx.shared_worktree_access(); let repo = ctx.repo.get()?; + // A non-conflicted commit has no conflicts — a valid answer for a read + // API. Clients routinely race a query against a commit that was just + // resolved, and that must not surface as an error. + if !but_core::Commit::from_id(commit_id.attach(&repo))?.is_conflicted() { + return Ok(CommitConflicts { + commit_id, + files: Vec::new(), + }); + } let request = context::build_request(&repo, commit_id)?; let ours_tree = repo.find_tree(request.ours_tree_id)?; let theirs_tree = repo.find_tree(request.theirs_tree_id)?; @@ -150,13 +162,16 @@ pub enum HunkResolution { /// Replace the conflict with this content (for mixed resolutions; an /// empty string deletes the conflicted region). Content(String), + /// Let the configured AI model merge this conflict. The model sees only + /// the AI-addressed hunks and its output is applied like [`Self::Content`]. + Ai, } #[cfg(feature = "export-schema")] but_schemars::register_sdk_type!(HunkResolution); /// One conflict to resolve, addressed by path and 1-based hunk index as -/// returned by [`commit_conflicts()`]. +/// returned by `commit_conflicts()`. #[derive(Debug, Clone, Serialize, Deserialize)] #[cfg_attr(feature = "export-schema", derive(schemars::JsonSchema))] #[serde(rename_all = "camelCase")] @@ -195,6 +210,9 @@ pub struct HunkResolutionResult { pub new_commit: gix::ObjectId, /// How many conflicts were resolved. pub resolved: usize, + /// Whether the fully resolved commit ended up with the same tree as its + /// parent — the resolutions dropped all of its changes. + pub commit_emptied: bool, /// The conflicts that remain, per file. Empty when the commit is fully /// resolved. pub remaining: Vec, @@ -209,13 +227,51 @@ pub struct HunkResolutionResult { /// into a normal commit. Either way the commit id changes — address follow-up /// resolutions to the returned `new_commit`. An oplog snapshot records an undo /// point. Nothing is written if any spec fails validation. +/// +/// [`HunkResolution::Ai`] specs are sent to the configured LLM first (no +/// worktree lock is held during the model call); AI configuration is only +/// required when such a spec is present. #[but_api(try_from = crate::resolve::json::HunkResolutionResult)] #[instrument(skip(specs), err(Debug))] pub fn resolve_commit_conflict_hunks( ctx: &mut but_ctx::Context, commit_id: gix::ObjectId, specs: Vec, - dry_run: DryRun, +) -> anyhow::Result { + let llm = specs + .iter() + .any(|spec| matches!(spec.resolution, HunkResolution::Ai)) + .then(|| -> anyhow::Result<_> { + let repo = ctx.repo.get()?; + let config = repo.config_snapshot(); + LLMProvider::from_git_config(config.plumbing()).context( + "AI is not configured. Configure an AI provider in the GitButler settings first.", + ) + }) + .transpose()?; + resolve_commit_conflict_hunks_with(ctx, commit_id, specs, move |request| { + let llm = llm.as_ref().expect("only called when an Ai spec exists"); + let model = llm.model_or_default(); + llm.structured_output::( + SYSTEM_PROMPT, + vec![ChatMessage::User(prompt::render_user_message(request))], + &model, + )? + .context("The AI model returned no response") + }) +} + +/// Like `resolve_commit_conflict_hunks()`, but with the model call injected +/// as `resolve`, so tests can supply AI resolutions without network access. +/// +/// `resolve` is only invoked when `specs` contains [`HunkResolution::Ai`], +/// with a request narrowed to just those hunks; a failed call or response is +/// retried once. +pub fn resolve_commit_conflict_hunks_with( + ctx: &mut but_ctx::Context, + commit_id: gix::ObjectId, + mut specs: Vec, + resolve: impl Fn(&ResolutionRequest) -> anyhow::Result, ) -> anyhow::Result { if specs.is_empty() { bail!("No resolutions were provided"); @@ -225,9 +281,15 @@ pub fn resolve_commit_conflict_hunks( let repo = ctx.repo.get()?; context::build_request(&repo, commit_id)? }; + let any_ai = resolve_ai_specs(&request, &mut specs, &resolve)?; let picks = apply::validate_specs(&request, &specs)?; // Every spec addressed a distinct hunk, or validation would have failed. let resolved = specs.len(); + let operation = if any_ai { + OperationKind::ResolveConflictsAi + } else { + OperationKind::ResolveConflicts + }; let mut guard = ctx.exclusive_worktree_access(); ctx.invalidate_workspace_cache()?; @@ -235,8 +297,8 @@ pub fn resolve_commit_conflict_hunks( ctx, &request, &picks, - OperationKind::ResolveConflicts, - dry_run, + operation, + DryRun::No, guard.write_permission(), )?; @@ -244,11 +306,86 @@ pub fn resolve_commit_conflict_hunks( commit_id: request.commit_id, new_commit: applied.new_commit, resolved, + commit_emptied: applied.commit_emptied, remaining: applied.remaining, workspace: applied.workspace, }) } +/// Replace every [`HunkResolution::Ai`] spec with the model's merged content +/// for that hunk, obtained via one `resolve` call over a request narrowed to +/// the AI-addressed hunks. Returns whether any spec was AI-resolved. +fn resolve_ai_specs( + request: &ResolutionRequest, + specs: &mut [ResolutionSpec], + resolve: &impl Fn(&ResolutionRequest) -> anyhow::Result, +) -> anyhow::Result { + let files_by_path = apply::index_files_by_path(request)?; + // Per request-file index, the AI-addressed hunks as (spec index, 0-based + // hunk index), ordered by hunk so the narrowed request stays in file order. + let mut targets_per_file: BTreeMap> = BTreeMap::new(); + for (spec_index, spec) in specs.iter().enumerate() { + if !matches!(spec.resolution, HunkResolution::Ai) { + continue; + } + let (file_index, hunk_index) = + apply::locate_hunk(request, &files_by_path, &spec.path, spec.hunk)?; + targets_per_file + .entry(file_index) + .or_default() + .push((spec_index, hunk_index)); + } + if targets_per_file.is_empty() { + return Ok(false); + } + for targets in targets_per_file.values_mut() { + targets.sort_by_key(|&(_, hunk_index)| hunk_index); + } + + let narrowed = ResolutionRequest { + commit_id: request.commit_id, + commit_message: request.commit_message.clone(), + parent_message: request.parent_message.clone(), + base_tree_id: request.base_tree_id, + ours_tree_id: request.ours_tree_id, + theirs_tree_id: request.theirs_tree_id, + files: targets_per_file + .iter() + .map(|(&file_index, targets)| { + let file = &request.files[file_index]; + FileConflict { + path: file.path.clone(), + rela_path: file.rela_path.clone(), + entry_kind: file.entry_kind, + merged_text: file.merged_text.clone(), + hunks: targets + .iter() + .map(|&(_, hunk_index)| file.hunks[hunk_index].clone()) + .collect(), + } + }) + .collect(), + }; + + let picks = retry_once(|| { + let response = resolve(&narrowed)?; + let (picks, _files) = validate_ai_response(&narrowed, &response)?; + Ok(picks) + })?; + + // The narrowed files and the validated picks are aligned index-for-index, + // and within a file the picks' 0..n keys match the sorted targets. + for (targets, file_picks) in targets_per_file.values().zip(picks) { + for (&(spec_index, _), (_, pick)) in targets.iter().zip(file_picks) { + let apply::HunkPick::Content(content) = pick else { + unreachable!("validated AI responses only produce content picks"); + }; + specs[spec_index].resolution = HunkResolution::Content(content); + } + } + Ok(true) +} + /// How one conflicted file was resolved, for display to the user. #[derive(Debug, Clone, Serialize)] #[cfg_attr(feature = "export-schema", derive(schemars::JsonSchema))] @@ -321,7 +458,7 @@ pub fn resolve_commit_conflicts_ai( }) } -/// Like [`resolve_commit_conflicts_ai()`], but with the model call injected as +/// Like `resolve_commit_conflicts_ai()`, but with the model call injected as /// `resolve`, so tests and other frontends can supply resolutions without /// network access. /// @@ -343,21 +480,11 @@ pub fn resolve_commit_conflicts_with( // The model call happens without any worktree lock; the request is plain // data and the apply step below re-reads the workspace under the exclusive // lock, so it fails closed if the commit changed in the meantime. - let attempt = || -> anyhow::Result<_> { + let ((picks, files), summary) = retry_once(|| { let response = resolve(&request)?; let validated = validate_ai_response(&request, &response)?; Ok((validated, response.summary)) - }; - let ((picks, files), summary) = match attempt() { - Ok(outcome) => outcome, - Err(first_failure) => { - tracing::warn!( - error = ?first_failure, - "AI conflict-resolution attempt failed, retrying once" - ); - attempt()? - } - }; + })?; let mut guard = ctx.exclusive_worktree_access(); // The workspace graph may have been cached before or during the model @@ -382,6 +509,18 @@ pub fn resolve_commit_conflicts_with( }) } +/// Run `attempt`, and once more if it fails — covering truncated or malformed +/// model output as well as responses that do not validate against the request. +fn retry_once(attempt: impl Fn() -> anyhow::Result) -> anyhow::Result { + attempt().or_else(|first_failure| { + tracing::warn!( + error = ?first_failure, + "AI conflict-resolution attempt failed, retrying once" + ); + attempt() + }) +} + /// Check the model response against the request and translate it into /// full-coverage picks plus the per-file display payload. Any mismatch fails /// validation so the caller can retry the model once before giving up. @@ -517,6 +656,9 @@ pub mod json { pub new_commit: HexHash, /// How many conflicts were resolved. pub resolved: usize, + /// Whether the fully resolved commit ended up with the same tree as + /// its parent — the resolutions dropped all of its changes. + pub commit_emptied: bool, /// The conflicts that remain, per file. pub remaining: Vec, /// Workspace state after the apply. @@ -534,6 +676,7 @@ pub mod json { commit_id, new_commit, resolved, + commit_emptied, remaining, workspace, } = value; @@ -541,6 +684,7 @@ pub mod json { commit_id: commit_id.into(), new_commit: new_commit.into(), resolved, + commit_emptied, remaining, workspace: workspace.try_into()?, }) diff --git a/crates/but-api/tests/api/resolve_hunks.rs b/crates/but-api/tests/api/resolve_hunks.rs index 1225d4610b5..996423bf2e2 100644 --- a/crates/but-api/tests/api/resolve_hunks.rs +++ b/crates/but-api/tests/api/resolve_hunks.rs @@ -2,7 +2,6 @@ use anyhow::Result; use but_api::resolve::{ HunkResolution, ResolutionSpec, commit_conflicts, resolve_commit_conflict_hunks, }; -use but_core::DryRun; use gitbutler_oplog::OplogExt as _; use gix::prelude::ObjectIdExt as _; @@ -75,7 +74,6 @@ fn partial_resolution_narrows_the_conflict_then_completes() -> Result<()> { 1, HunkResolution::Content("line two merged".into()), )], - DryRun::No, )?; assert_eq!(first.resolved, 1); assert_eq!(first.remaining.len(), 1); @@ -106,6 +104,16 @@ fn partial_resolution_narrows_the_conflict_then_completes() -> Result<()> { auto_blob.data.as_slice(), b"line one\nline two merged\nline three\nline four\nline five\nline six changed by the new base\nline seven\n" ); + // The stored base stays untouched: the resolution lives in ours and + // theirs, so a later re-pick onto changed parents replays it instead + // of dropping it as a region the commit does not change. + let base_blob = repo + .rev_parse_single(format!("{}:.conflict-base-0/conflict", first.new_commit).as_str())? + .object()?; + assert_eq!( + base_blob.data.as_slice(), + b"line one\nline two\nline three\nline four\nline five\nline six\nline seven\n" + ); // The descendant sits on the narrowed commit. let descendant = repo .rev_parse_single("refs/heads/branchy")? @@ -135,10 +143,10 @@ fn partial_resolution_narrows_the_conflict_then_completes() -> Result<()> { &mut ctx, first.new_commit, vec![spec("conflict", 1, HunkResolution::Theirs)], - DryRun::No, )?; assert_eq!(second.resolved, 1); assert!(second.remaining.is_empty()); + assert!(!second.commit_emptied, "the commit keeps its own changes"); { let repo = ctx.repo.get()?; @@ -187,10 +195,13 @@ fn taking_one_side_for_all_hunks_resolves_the_commit() -> Result<()> { spec("conflict", 1, HunkResolution::Ours), spec("conflict", 2, HunkResolution::Ours), ], - DryRun::No, )?; assert_eq!(result.resolved, 2); assert!(result.remaining.is_empty()); + assert!( + result.commit_emptied, + "keeping the base everywhere leaves the commit without changes of its own" + ); let repo = ctx.repo.get()?; let resolved = but_core::Commit::from_id(result.new_commit.attach(&repo))?; @@ -213,13 +224,97 @@ fn invalid_specs_change_nothing() -> Result<()> { &mut ctx, conflicted_commit, vec![spec("conflict", 3, HunkResolution::Ours)], - DryRun::No, ) .unwrap_err(); - assert!(err.to_string().contains("has conflicts 1..2"), "{err}"); + assert!( + err.to_string() + .contains("has 2 conflicts, but conflict 3 was addressed"), + "{err}" + ); let repo = ctx.repo.get()?; let commit = but_core::Commit::from_id(conflicted_commit.attach(&repo))?; assert!(commit.is_conflicted(), "nothing may be written on failure"); Ok(()) } + +#[test] +fn ai_specs_are_narrowed_to_their_hunks_and_applied() -> Result<()> { + use but_api::resolve::{ + FileResolution, HunkContent, ResolutionResponse, resolve_commit_conflict_hunks_with, + }; + + let (mut ctx, conflicted_commit, _tmp) = conflicted_context()?; + + // One AI-resolved and one side-picked conflict in a single call. + let result = resolve_commit_conflict_hunks_with( + &mut ctx, + conflicted_commit, + vec![ + spec("conflict", 1, HunkResolution::Ai), + spec("conflict", 2, HunkResolution::Theirs), + ], + |request| { + // The model must only see the AI-addressed hunk. + assert_eq!(request.files.len(), 1); + assert_eq!(request.files[0].hunks.len(), 1); + assert_eq!( + request.files[0].hunks[0].ours, + "line two changed by the new base" + ); + Ok(ResolutionResponse { + summary: None, + resolutions: vec![FileResolution { + path: "conflict".into(), + hunks: vec![HunkContent { + resolved_content: "line two merged by ai".into(), + }], + reasoning: "combined both sides".into(), + }], + }) + }, + )?; + assert_eq!(result.resolved, 2); + assert!(result.remaining.is_empty()); + + let repo = ctx.repo.get()?; + let resolved = but_core::Commit::from_id(result.new_commit.attach(&repo))?; + assert!(!resolved.is_conflicted()); + let blob = repo + .rev_parse_single(format!("{}:conflict", result.new_commit).as_str())? + .object()?; + assert_eq!( + blob.data.as_slice(), + b"line one\nline two merged by ai\nline three\nline four\nline five\nline six changed by this commit\nline seven\n" + ); + + // A mixed AI + manual apply records an AI undo point. + let ai_snapshots = ctx + .snapshots_iter(None, Vec::new(), None)? + .filter_map(Result::ok) + .filter(|snapshot| { + snapshot.details.as_ref().is_some_and(|details| { + matches!( + details.operation, + but_oplog::legacy::OperationKind::ResolveConflictsAi + ) + }) + }) + .count(); + assert_eq!(ai_snapshots, 1); + Ok(()) +} + +#[test] +fn conflicts_of_a_normal_commit_are_empty_not_an_error() -> Result<()> { + let (ctx, _conflicted_commit, _tmp) = conflicted_context()?; + let normal_commit = { + let repo = ctx.repo.get()?; + repo.rev_parse_single("refs/heads/main")?.detach() + }; + + let conflicts = commit_conflicts(&ctx, normal_commit)?; + assert_eq!(conflicts.commit_id, normal_commit); + assert!(conflicts.files.is_empty()); + Ok(()) +} diff --git a/crates/but-api/tests/fixtures/scenario/resolve-ai-conflicted-commit.sh b/crates/but-api/tests/fixtures/scenario/resolve-ai-conflicted-commit.sh index 0706616d151..914f6703dcf 100755 --- a/crates/but-api/tests/fixtures/scenario/resolve-ai-conflicted-commit.sh +++ b/crates/but-api/tests/fixtures/scenario/resolve-ai-conflicted-commit.sh @@ -38,6 +38,14 @@ theirEntries = [ "conflict" ] EOF ) +# The commit the conflicted one was rebased onto: its tree is the "ours" side, +# exactly like the rebase engine leaves it. +git read-tree --empty +git update-index --add --cacheinfo 100644 "$unrelated_blob" "file" +git update-index --add --cacheinfo 100644 "$ours_blob" "conflict" +new_base_tree=$(git write-tree) +new_base_commit=$(git commit-tree "$new_base_tree" -p "$(git rev-parse HEAD)" -m "new base") + git read-tree --empty git update-index --add --cacheinfo 100644 "$unrelated_blob" ".auto-resolution/file" git update-index --add --cacheinfo 100644 "$ours_blob" ".auto-resolution/conflict" @@ -52,7 +60,7 @@ conflict_tree=$(git write-tree) conflict_commit=$(git hash-object -wt commit --stdin < 1730625617 +0100 committer GitButler 1730625617 +0100 gitbutler-headers-version 2 diff --git a/crates/but/skill/SKILL.md b/crates/but/skill/SKILL.md index 823aea1cad6..d6bd297d54a 100644 --- a/crates/but/skill/SKILL.md +++ b/crates/but/skill/SKILL.md @@ -224,6 +224,7 @@ Conflicts do not interrupt operations in GitButler: a rebase always completes, a 2. Apply one resolution per command: - Merged/mixed content (the common case — combine the intent of both sides): pipe it to `but resolve apply :` via stdin (heredoc) or pass `--file `. The content replaces the whole conflicted region; never include conflict markers. - Take a side entirely: `but resolve apply : --ours` or `--theirs` (bare `` applies the side to all conflicts in that file). + - Delegate one conflict to the configured AI model: `but resolve apply : --ai` — prefer your own merged content when you have the context. - `apply` targets the same default commit as `conflicts`; when more than one branch is conflicted, pin it with `--commit `. 3. Go back to step 1 until it reports no conflicts. Commit ids are not stable here — every apply rewrites the commit — but branch names are, so address everything by branch and never bookkeep commit ids. Partial progress is fine: a commit with unresolved conflicts left stays `{conflicted}` with exactly those conflicts. diff --git a/crates/but/src/args/resolve.rs b/crates/but/src/args/resolve.rs index 50000680ec1..367ae4929f7 100644 --- a/crates/but/src/args/resolve.rs +++ b/crates/but/src/args/resolve.rs @@ -28,11 +28,14 @@ pub enum Subcommands { #[clap(long)] commit: Option, /// Take the ours side: the new base the commit was rebased onto. - #[clap(long, conflicts_with_all = ["theirs", "file"])] + #[clap(long, conflicts_with_all = ["theirs", "file", "ai"])] ours: bool, /// Take the theirs side: the commit's own version. - #[clap(long, conflicts_with = "file")] + #[clap(long, conflicts_with_all = ["file", "ai"])] theirs: bool, + /// Let the configured AI model merge the targeted conflicts. + #[clap(long, conflicts_with = "file")] + ai: bool, /// Read the replacement content from this file (otherwise from stdin). #[clap(long, short = 'F')] file: Option, diff --git a/crates/but/src/command/legacy/resolve.rs b/crates/but/src/command/legacy/resolve.rs index 93f21c31171..933edf8a39d 100644 --- a/crates/but/src/command/legacy/resolve.rs +++ b/crates/but/src/command/legacy/resolve.rs @@ -31,7 +31,9 @@ pub(crate) fn handle( ) -> Result<()> { if ai { if cmd.is_some() { - bail!("--ai cannot be combined with a resolve subcommand"); + bail!( + "--ai cannot be combined with a resolve subcommand. For one conflict, use `but resolve apply [:] --ai` instead." + ); } return resolve_with_ai(ctx, out, commit_id.as_deref()); } @@ -42,8 +44,26 @@ pub(crate) fn handle( commit, ours, theirs, + ai, file, - }) => apply_resolutions(ctx, out, commit.as_deref(), &target, ours, theirs, file), + }) => { + let flags = ResolutionFlags { + ours, + theirs, + ai, + file, + }; + // Lock stdin at the CLI boundary; the handler reads it only for a + // piped mixed-content resolution. + apply_resolutions( + ctx, + out, + commit.as_deref(), + &target, + flags, + std::io::stdin().lock(), + ) + } Some(Subcommands::Status) => show_status(ctx, out), Some(Subcommands::Finish) => finish_resolution(ctx, out), Some(Subcommands::Cancel { force }) => cancel_resolution(ctx, out, force), @@ -510,6 +530,26 @@ fn list_conflicts(ctx: &mut Context, out: &mut OutputChannel, commit: Option<&st }; let conflicts = but_api::resolve::commit_conflicts(ctx, target.commit_oid)?; + if conflicts.files.is_empty() { + if let Some(human_out) = out.for_human() { + let repo = ctx.repo.get()?; + writeln!( + human_out, + "{} {} {}", + t.important.paint("Commit"), + t.commit_id + .paint(shorten_object_id(&repo, target.commit_oid)), + t.success.paint("is not conflicted.") + )?; + } + if let Some(json_out) = out.for_json() { + json_out.write_value(serde_json::json!({ + "commit_id": target.commit_oid.to_string(), + "files": [], + }))?; + } + return Ok(()); + } if let Some(human_out) = out.for_human() { let repo = ctx.repo.get()?; @@ -563,12 +603,19 @@ fn list_conflicts(ctx: &mut Context, out: &mut OutputChannel, commit: Option<&st )) )?; } + // With several conflicted commits, a bare `apply` targets the default + // commit, not necessarily the one listed here — pin it in the hint. + let commit_flag = (target.other_conflicted > 0) + .then_some(target.branch.as_deref()) + .flatten() + .map(|branch| format!(" --commit {branch}")) + .unwrap_or_default(); writeln!( human_out, "{}", - t.hint.paint( - "Resolve with `but resolve apply [:] --ours|--theirs` or pipe mixed content into `but resolve apply :`. `but resolve --ai` resolves everything at once." - ) + t.hint.paint(format!( + "Resolve with `but resolve apply [:]{commit_flag} --ours|--theirs` or pipe mixed content into `but resolve apply :{commit_flag}`. `but resolve --ai` resolves everything at once." + )) )?; } @@ -585,17 +632,31 @@ fn list_conflicts(ctx: &mut Context, out: &mut OutputChannel, commit: Option<&st /// Apply resolutions to one conflict (`:`) or to every conflict of a /// file, without entering resolution mode. +/// How `but resolve apply` was asked to resolve, straight from the CLI flags. +struct ResolutionFlags { + ours: bool, + theirs: bool, + ai: bool, + file: Option, +} + fn apply_resolutions( ctx: &mut Context, out: &mut OutputChannel, commit: Option<&str>, target: &str, - ours: bool, - theirs: bool, - file: Option, + flags: ResolutionFlags, + mut read: impl std::io::Read, ) -> Result<()> { use but_api::resolve::{HunkResolution, ResolutionSpec}; + let ResolutionFlags { + ours, + theirs, + ai, + file, + } = flags; + let t = theme::get(); let mode = operating_mode(ctx)?.operating_mode; if matches!(mode, OperatingMode::Edit(_)) { @@ -622,20 +683,20 @@ fn apply_resolutions( HunkResolution::Ours } else if theirs { HunkResolution::Theirs + } else if ai { + HunkResolution::Ai } else { let content = match &file { Some(file) => std::fs::read_to_string(file) .with_context(|| format!("Failed to read {}", file.display()))?, None => { - use std::io::{IsTerminal, Read}; - let mut stdin = std::io::stdin(); - if stdin.is_terminal() { + if out.can_prompt() { bail!( - "Provide the replacement content via --file or stdin, or take a side with --ours/--theirs." + "Provide the replacement content via --file or stdin, or take a side with --ours/--theirs/--ai." ); } let mut content = String::new(); - stdin.read_to_string(&mut content)?; + std::io::Read::read_to_string(&mut read, &mut content)?; if content.is_empty() { bail!( "stdin was empty. To intentionally delete the conflicted region, pass --file with an empty file." @@ -653,17 +714,22 @@ fn apply_resolutions( (Some(hunk), _) => vec![hunk], (None, _) => { let conflicts = but_api::resolve::commit_conflicts(ctx, commit_oid)?; - // Request paths are canonical; accept the common `./`-prefixed - // spelling the API would accept too. - let lookup = path.trim().trim_start_matches("./"); + if conflicts.files.is_empty() { + bail!("Commit {commit_oid} is not conflicted."); + } + // Match with the same normalization the API applies, so spellings + // it would accept (`./`, backslashes, duplicate slashes) resolve. + let lookup = but_api::resolve::normalize_path(path); let total_hunks = conflicts .files .iter() - .find(|conflicted| conflicted.path == lookup) + .find(|conflicted| but_api::resolve::normalize_path(&conflicted.path) == lookup) .map(|conflicted| conflicted.hunks.len()) .with_context(|| format!("\"{path}\" is not a conflicted file of this commit"))?; match &resolution { - HunkResolution::Ours | HunkResolution::Theirs => (1..=total_hunks).collect(), + HunkResolution::Ours | HunkResolution::Theirs | HunkResolution::Ai => { + (1..=total_hunks).collect() + } HunkResolution::Content(_) if total_hunks == 1 => vec![1], HunkResolution::Content(_) => bail!( "\"{path}\" has {total_hunks} conflicts; content applies to one at a time, use \"{path}:\"." @@ -680,12 +746,7 @@ fn apply_resolutions( }) .collect(); - let result = but_api::resolve::resolve_commit_conflict_hunks( - ctx, - commit_oid, - specs, - but_core::DryRun::No, - )?; + let result = but_api::resolve::resolve_commit_conflict_hunks(ctx, commit_oid, specs)?; if let Some(human_out) = out.for_human() { let repo = ctx.repo.get()?; @@ -712,6 +773,14 @@ fn apply_resolutions( t.success .paint("All conflicts in this commit are resolved.") )?; + if result.commit_emptied { + writeln!( + human_out, + "{}", + t.attention + .paint("The commit is now empty — the kept content matches its parent.") + )?; + } drop(repo); let more = oldest_conflicted_commit(ctx)?.is_some(); if more { @@ -750,6 +819,7 @@ fn apply_resolutions( "commit_id": result.commit_id.to_string(), "new_commit_id": result.new_commit.to_string(), "resolved": result.resolved, + "commit_emptied": result.commit_emptied, "remaining": result.remaining, }))?; } diff --git a/packages/but-sdk/src/generated/graph/index.d.ts b/packages/but-sdk/src/generated/graph/index.d.ts index f603c0a14c9..feaaad64564 100644 --- a/packages/but-sdk/src/generated/graph/index.d.ts +++ b/packages/but-sdk/src/generated/graph/index.d.ts @@ -1934,6 +1934,8 @@ export type HunkResolution = { } | { type: "content"; subject: string; +} | { + type: "ai"; }; /** JSON transport type for the outcome of applying per-hunk resolutions. */ @@ -1944,6 +1946,11 @@ export type HunkResolutionResult = { newCommit: string; /** How many conflicts were resolved. */ resolved: number; + /** + * Whether the fully resolved commit ended up with the same tree as + * its parent — the resolutions dropped all of its changes. + */ + commitEmptied: boolean; /** The conflicts that remain, per file. */ remaining: Array; /** Workspace state after the apply. */ @@ -2400,7 +2407,7 @@ export type ResolutionApproach = { /** * One conflict to resolve, addressed by path and 1-based hunk index as - * returned by [`commit_conflicts()`]. + * returned by `commit_conflicts()`. */ export type ResolutionSpec = { /** The repo-relative path of the conflicted file. */ diff --git a/packages/but-sdk/src/generated/linear/index.d.ts b/packages/but-sdk/src/generated/linear/index.d.ts index b9dba65ddf3..5f5839d339b 100644 --- a/packages/but-sdk/src/generated/linear/index.d.ts +++ b/packages/but-sdk/src/generated/linear/index.d.ts @@ -1934,6 +1934,8 @@ export type HunkResolution = { } | { type: "content"; subject: string; +} | { + type: "ai"; }; /** JSON transport type for the outcome of applying per-hunk resolutions. */ @@ -1944,6 +1946,11 @@ export type HunkResolutionResult = { newCommit: string; /** How many conflicts were resolved. */ resolved: number; + /** + * Whether the fully resolved commit ended up with the same tree as + * its parent — the resolutions dropped all of its changes. + */ + commitEmptied: boolean; /** The conflicts that remain, per file. */ remaining: Array; /** Workspace state after the apply. */ @@ -2400,7 +2407,7 @@ export type ResolutionApproach = { /** * One conflict to resolve, addressed by path and 1-based hunk index as - * returned by [`commit_conflicts()`]. + * returned by `commit_conflicts()`. */ export type ResolutionSpec = { /** The repo-relative path of the conflicted file. */