Treat empty or invalid commit SHAs at UserMergeToRef RPCosw-treat-empty-commit-shas-merge-to-ref

It's possible that Gitaly sends an empty SHA when merging
to ref. It happens given at Gitaly we rescue a
`Gitlab::Git::CommitError` (which happens when conflicting).
Given the way protobuffers work, it converts the response
into a empty string.

5 files changed, 46 insertions, 12 deletions

diff --git a/app/models/repository.rb b/app/models/repository.rb
index 187382ad182..8b5202ace3e 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -841,8 +841,9 @@ class Repository
 
   def merge_to_ref(user, source_sha, merge_request, target_ref, message, first_parent_ref)
     branch = merge_request.target_branch
+    newrev = raw.merge_to_ref(user, source_sha, branch, target_ref, message, first_parent_ref)
 
-    raw.merge_to_ref(user, source_sha, branch, target_ref, message, first_parent_ref)
+    newrev.presence
   end
 
   def delete_refs(*ref_names)
diff --git a/app/services/merge_requests/merge_to_ref_service.rb b/app/services/merge_requests/merge_to_ref_service.rb
index 37b5805ae7e..219b06aeeed 100644
--- a/app/services/merge_requests/merge_to_ref_service.rb
+++ b/app/services/merge_requests/merge_to_ref_service.rb
@@ -20,12 +20,13 @@ module MergeRequests
 
       commit_id = commit
 
-      raise_error('Conflicts detected during merge') unless commit_id
+      unless commit_id && ref_head = project.commit(commit_id)
+        raise_error('Conflicts detected during merge')
+      end
 
-      commit = project.commit(commit_id)
-      target_id, source_id = commit.parent_ids
+      target_id, source_id = ref_head.parent_ids
 
-      success(commit_id: commit.id,
+      success(commit_id: ref_head.id,
               target_id: target_id,
               source_id: source_id)
     rescue MergeError, ArgumentError => error
diff --git a/changelogs/unreleased/osw-treat-empty-commit-shas-merge-to-ref.yml b/changelogs/unreleased/osw-treat-empty-commit-shas-merge-to-ref.yml
new file mode 100644
index 00000000000..e369de1daa2
--- /dev/null
+++ b/changelogs/unreleased/osw-treat-empty-commit-shas-merge-to-ref.yml
@@ -0,0 +1,5 @@
+---
+title: Treat empty or invalid commit SHAs at UserMergeToRef RPC
+merge_request: 30707
+author:
+type: fixed
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 12dff440ce2..11c46c4fb8c 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -1435,6 +1435,21 @@ describe Repository do
       expect(merge_commit.author_email).to eq(user.commit_email)
       expect(repository.blob_at(merge_commit.id, 'files/ruby/feature.rb')).to be_present
     end
+
+    context 'when empty new rev' do
+      it 'returns nil' do
+        expect(repository.raw_repository).to receive(:merge_to_ref) { '' }
+
+        merge_commit_id = repository.merge_to_ref(user,
+                                                  merge_request.diff_head_sha,
+                                                  merge_request,
+                                                  merge_request.merge_ref_path,
+                                                  'Custom message',
+                                                  merge_request.target_branch_ref)
+
+        expect(merge_commit_id).to be_nil
+      end
+    end
   end
 
   describe '#ff_merge' do
diff --git a/spec/services/merge_requests/merge_to_ref_service_spec.rb b/spec/services/merge_requests/merge_to_ref_service_spec.rb
index 758679edc45..7d415da3b82 100644
--- a/spec/services/merge_requests/merge_to_ref_service_spec.rb
+++ b/spec/services/merge_requests/merge_to_ref_service_spec.rb
@@ -152,18 +152,30 @@ describe MergeRequests::MergeToRefService do
         it_behaves_like 'successfully evaluates pre-condition checks'
       end
 
-      context 'when MR is not mergeable to ref' do
+      context 'when MR is not mergeable to ref (conflicts)' do
         let(:merge_method) { :merge }
+        let(:error_message) { 'Conflicts detected during merge' }
 
-        it 'returns error' do
-          allow(project).to receive_message_chain(:repository, :merge_to_ref) { nil }
+        context 'when returned commit ID is nil' do
+          it 'returns error' do
+            allow(project.repository).to receive(:merge_to_ref) { nil }
 
-          error_message = 'Conflicts detected during merge'
+            result = service.execute(merge_request)
 
-          result = service.execute(merge_request)
+            expect(result[:status]).to eq(:error)
+            expect(result[:message]).to eq(error_message)
+          end
+        end
+
+        context 'when returned commit ID is cannot be found at repo' do
+          it 'returns error' do
+            allow(project.repository).to receive(:merge_to_ref) { 'invalid-ref' }
 
-          expect(result[:status]).to eq(:error)
-          expect(result[:message]).to eq(error_message)
+            result = service.execute(merge_request)
+
+            expect(result[:status]).to eq(:error)
+            expect(result[:message]).to eq(error_message)
+          end
         end
       end
     end