feat(source-control): ✨ Add git merge conflict detection and diff viewing - #101
Conversation
Add comprehensive support for detecting and displaying git merge/rebase conflicts in the source control panel: - Separate conflicted files from staged/unstaged/untracked lists into dedicated conflicted_files field in Git snapshot - Add GitFileState.Conflicted variant and is_conflicted status flag to track file conflict state - Create ConflictGroup component to display conflicting files with "C" badge indicator in warning color - Implement git_get_conflict_diff command to view conflict markers (<<<<<<<, =======, >>>>>>>) in diff preview - Add i18n keys for "Conflicts" section label in English and Chinese Users can now easily identify files with merge conflicts and view the full conflict content in the diff preview panel.
AI Code Review SummaryPR: #101 (feat(source-control): ✨ Add git merge conflict detection and diff viewing) Overall AssessmentDetected 5 actionable findings, prioritize CRITICAL/HIGH before merge. Major Findings by Severity
Actionable Suggestions
Potential Risks
Test Suggestions
File-Level Coverage Notes
Inline Downgraded Items (processed but not inline)
Coverage Status
Uncovered list:
No-patch covered list:
Runtime/Budget
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| } | ||
|
|
||
| #[tauri::command] | ||
| pub async fn git_get_conflict_diff( |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| Ok(files) | ||
| } | ||
|
|
||
| fn collect_conflicted_files( |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
|
||
| void Promise.all([ | ||
| gitGetDiff(workspaceId, selection.path, selection.staged), | ||
| selection.isConflict |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| while let Some(conflict) = conflict_iter.next() { | ||
| let conflict = match conflict { | ||
| Ok(c) => c, | ||
| Err(_) => continue, |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| let staged_files = collect_staged_files(&repo, &repo_root, workspace_root)?; | ||
| let unstaged_files = collect_unstaged_files(&repo, &repo_root, workspace_root)?; | ||
| let untracked_files = collect_untracked_files(&repo, &repo_root, workspace_root)?; | ||
| let conflicted_files = collect_conflicted_files(&repo, &repo_root, workspace_root)?; |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| ); | ||
| } | ||
|
|
||
| function ConflictGroup({ |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| : null, | ||
| fileStatus.isUntracked ? "untracked" : null, | ||
| fileStatus.isIgnored ? "ignored" : null, | ||
| fileStatus.isConflicted ? "conflict" : null, |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| }); | ||
| } | ||
|
|
||
| export async function gitGetConflictDiff( |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| // conflict markers) and display it as all-new content. | ||
| // This gives users a clear view of the current state of the file | ||
| // including conflict markers (<<<<<<<, =======, >>>>>>>). | ||
| let hunks = build_added_file_fallback_hunks(&workspace_root, &workspace_relative); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| ); | ||
| } | ||
|
|
||
| function ConflictGroup({ |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
|
||
| void Promise.all([ | ||
| gitGetDiff(workspaceId, selection.path, selection.staged), | ||
| selection.isConflict |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| tracked: 2, | ||
| modified: 3, | ||
| untracked: 4, | ||
| conflicted: 5, |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| const isUntracked = node.gitState === "untracked"; | ||
| const badgeLabel = isUntracked ? "U" : isModified ? "M" : null; | ||
| const isConflicted = node.gitState === "conflicted"; | ||
| const badgeLabel = isConflicted ? "C" : isUntracked ? "U" : isModified ? "M" : null; |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| Ok(files) | ||
| } | ||
|
|
||
| fn collect_conflicted_files( |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| while let Some(conflict) = conflict_iter.next() { | ||
| let conflict = match conflict { | ||
| Ok(c) => c, | ||
| Err(_) => continue, |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
… warning log and fix "conflicted" string mismatch
| .await | ||
| } | ||
|
|
||
| #[tauri::command] |
There was a problem hiding this comment.
[HIGH] No integration test for new git_get_conflict_diff command
The new git_get_conflict_diff Tauri command has no integration test coverage, violating the project's testing guidelines for command behavior changes.
Suggestion: Add an integration test in src-tauri/tests/ (following the milestone naming pattern) that sets up a repo with a merge conflict and verifies the command returns the expected GitDiffDto with Unmerged status and conflict-marker hunks.
Risk: Regression in conflict diff rendering could go undetected; the command could fail silently or return malformed data without any automated safety net.
Confidence: 0.95
| Ok(files) | ||
| } | ||
|
|
||
| fn collect_conflicted_files( |
There was a problem hiding this comment.
[HIGH] No unit tests for collect_conflicted_files function
The collect_conflicted_files function has multiple branching paths and error-handling logic that are untested, including malformed entries and path resolution failures.
Suggestion: Add Rust unit tests covering: (1) repo with no conflicts returns empty vec, (2) repo with conflicts returns correct paths with Unmerged status, (3) malformed conflict entries are skipped with warning, (4) entries with no ancestor/ours/theirs are skipped, (5) paths outside workspace root are filtered out.
Risk: Silent data loss if conflict entries are unexpectedly skipped due to path resolution or encoding issues; users may not see all conflicted files.
Confidence: 0.92
| onToggleStage={handleToggleStage} | ||
| onToggleAll={handleToggleAll} | ||
| /> | ||
| {snapshot.conflictedFiles.length > 0 && ( |
There was a problem hiding this comment.
[HIGH] conflictedFiles accessed without optional chaining — potential runtime crash
snapshot.conflictedFiles is accessed without optional chaining at R1660 and R1663, while R1062 safely uses snapshot?.conflictedFiles. If the backend omits the field, the direct access will crash.
Suggestion: Use snapshot?.conflictedFiles at R1660-R1663 or provide a fallback default (e.g., snapshot.conflictedFiles?.length ?? 0 > 0). Add a unit test that renders GitPanel with a snapshot missing conflictedFiles to verify backwards compatibility.
Risk: Runtime TypeError when receiving a GitSnapshotDto that lacks the conflictedFiles property.
Confidence: 0.88
| })? | ||
| } | ||
|
|
||
| pub async fn get_conflict_diff( |
There was a problem hiding this comment.
[MEDIUM] get_conflict_diff missing test coverage for edge cases
The get_conflict_diff method has no test coverage for binary files, missing files, empty files, or truncation scenarios.
Suggestion: Add tests for: (1) binary conflict file producing is_binary:true with empty hunks, (2) conflict file that doesn't exist on disk (build_added_file_fallback_hunks failure path), (3) large conflict file truncation behavior, (4) empty conflict file.
Risk: Binary conflict files or missing workdir files could produce unexpected diff results that break the UI without any automated detection.
Confidence: 0.85
| onToggleStage={handleToggleStage} | ||
| onToggleAll={handleToggleAll} | ||
| /> | ||
| {snapshot.conflictedFiles.length > 0 && ( |
There was a problem hiding this comment.
[MEDIUM] Missing optional chaining on snapshot.conflictedFiles in ConflictGroup render
At R1660, snapshot.conflictedFiles.length is accessed without optional chaining, while the same property at R1062 uses snapshot?.conflictedFiles.length ?? 0. Since the optional chaining at R1062 implies snapshot can be null, the direct access at R1660 may crash.
Suggestion: Change snapshot.conflictedFiles.length > 0 to snapshot?.conflictedFiles.length or add a guard: (snapshot?.conflictedFiles?.length ?? 0) > 0.
Risk: Runtime TypeError crash if snapshot is null when the ConflictGroup section renders.
Confidence: 0.85
1 participant
Summary
git2index conflict iterator and expose them as a dedicatedconflictedFileslist inGitSnapshotDto"Conflicts"section in the Git panel UI with warning styling andCbadgeisConflictedtoGitFileStatusDtoandconflictedtoGitFileStatefor file tree overlayChanges
Backend (Rust / git2)
git_manager.rs: Addconflicted_filestoSnapshotParts,collect_conflicted_files()usingindex.conflicts()iterator, andget_conflict_diff()usingbuild_added_file_fallback_hunksgit_manager.rs: Filter conflicted paths from staged/unstaged/untracked lists viaHashSetdedupgit_manager.rs: AddConflictedtoGitFileStatepriority mapping andmap_overlay_statusgit.rs(model): Addconflicted_filestoGitSnapshotDto,is_conflictedtoGitFileStatusDto,Conflictedvariant toGitFileStategit.rs(commands): Exposegit_get_conflict_diffTauri commandlib.rs: Registergit_get_conflict_diffcommandindex_manager.rs: AddConflictedstate priorityFrontend (React / TypeScript)
api.ts: AddconflictedFilestoGitSnapshotDto,isConflictedtoGitFileStatusDto,"conflicted"toGitFileStatesource-control-panels.tsx: AddConflictGroupcomponent with warning styling; route conflict file clicks togitGetConflictDiff; includeisConflictflag inGitDiffSelectionbranch-selector.tsx/dashboard-workbench.tsx: WireconflictedFilesthrough propsproject-panel.tsx: DisplayCbadge for conflicted files in tree viewgit-commands.ts: AddgitGetConflictDiffIPC bridge"sourceControl.conflicts"/"sourceControl.statusConflict"keys (en + zh-CN)Test Plan
cargo checkpasses with zero errors/warningstsc --noEmitpasses with zero errorsCbadge shows in file tree for conflicted paths🤖 Generated with TiyCode