Draft: Group conflict files correctly when directory changes on target

DUO Created: To be evaluated by the Human (@marc_shaw)

ListConflictFiles incorrectly groups conflict stages when files are renamed, causing conflict_type to be null in GitLab Rails

Description

Summary

When a merge conflict involves a file/directory rename, ListConflictFiles in Gitaly incorrectly groups conflict stages, resulting in incomplete conflict data being sent to GitLab Rails. This causes the conflict_type field to be null in the API response instead of showing the correct conflict type (e.g., both_modified).

Root Cause

In internal/gitaly/service/conflicts/list_conflict_files.go (lines 82-97), the code groups conflict stages using conflictFile.FileName as the map key:

`for _, conflictFile := range mergeConflictErr.ConflictingFileInfo { val, ok := pathToConflict[conflictFile.FileName] // <-- BUG: Uses filename as key if !ok { val = &conflictHeader{} conflicts = append(conflicts, val) }

switch conflictFile.Stage {
case localrepo.MergeStageAncestor:
    val.ancestorPath = conflictFile.FileName
case localrepo.MergeStageOurs:
    val.ourPath = conflictFile.FileName
    val.ourMode = conflictFile.Mode
case localrepo.MergeStageTheirs:
    val.theirPath = conflictFile.FileName
}

pathToConflict[conflictFile.FileName] = val

} `

The problem: When a directory is renamed (e.g., /devops/dev/create/ → /devops/create/), Git's merge-tree reports different filenames for different stages:

  • Stage 1 (Ancestor): content/handbook/engineering/devops/dev/create/file.md
  • Stage 2 (Ours): content/handbook/engineering/devops/dev/create/file.md
  • Stage 3 (Theirs): content/handbook/engineering/devops/create/file.md

Since the map key is conflictFile.FileName, these three stages create three separate entries instead of being grouped together:

  • Entry 1: ancestorPath set, ourPath and theirPath empty
  • Entry 2: ourPath set, ancestorPath and theirPath empty
  • Entry 3: theirPath set, ancestorPath and ourPath empty

Impact on GitLab Rails

When Rails receives these incomplete conflict entries:

  1. The conflicts_with_types helper in app/helpers/diff_helper.rb builds a hash indexed by file.path
  2. Each incomplete entry has a different path, so they don't overwrite each other
  3. When the serializer looks up options[:conflicts][diff_file.new_path], it may:
    • Not find a match (returns null)
    • Find an incomplete entry with the wrong conflict_type
  4. The conflict_type field in the API response becomes null

Example MR

gitlab-com/content-sites/handbook!17166

In this MR:

  • The file content/handbook/engineering/devops/dev/create/engineering-managers/_index.md was renamed to content/handbook/engineering/devops/create/engineering-managers/_index.md in the target branch
  • Both branches modified the file, creating a both_modified conflict
  • But conflict_type is returned as null in the API

Proposed Fix

Replace the filename-based grouping with sequential processing that relies on Git's guarantee that stages for the same conflict appear consecutively:

`var conflicts []*conflictHeader var currentConflict *conflictHeader seenStages := make(map[localrepo.MergeStage]bool)

for _, conflictFile := range mergeConflictErr.ConflictingFileInfo { // Start a new conflict if: // 1. This is the first file, OR // 2. We see stage 1 (ancestor) again after having seen it before, OR // 3. We see a stage we've already processed (indicates new conflict) if currentConflict == nil || (conflictFile.Stage == localrepo.MergeStageAncestor && seenStages[localrepo.MergeStageAncestor]) || seenStages[conflictFile.Stage] { currentConflict = &conflictHeader{} conflicts = append(conflicts, currentConflict) seenStages = make(map[localrepo.MergeStage]bool) }

switch conflictFile.Stage {
case localrepo.MergeStageAncestor:
    currentConflict.ancestorPath = conflictFile.FileName
case localrepo.MergeStageOurs:
    currentConflict.ourPath = conflictFile.FileName
    currentConflict.ourMode = conflictFile.Mode
case localrepo.MergeStageTheirs:
    currentConflict.theirPath = conflictFile.FileName
}

seenStages[conflictFile.Stage] = true

} `

This ensures all three stages are grouped into a single conflictHeader with all paths properly set, even when filenames differ across stages.

Related Code

  • Gitaly: internal/gitaly/service/conflicts/list_conflict_files.go
  • GitLab Rails:
    • app/helpers/diff_helper.rb (conflicts_with_types method)
    • app/serializers/concerns/diff_file_conflict_type.rb
    • lib/gitlab/conflict/file.rb (conflict_type method)

Labels

~"type::bug" ~"group::code review" ~"devops::create" ~"section::dev" ~"backend"

Edited by Marc Shaw