Approvals reset when using 'Rebase without pipeline' button due to race condition

Summary

Merge request approvals are unexpectedly reset when using the "Rebase without pipeline" button in the GitLab UI due to a race condition between the rebase operation and the approval reset check.

Steps to reproduce

1. Create a feature branch and push it to the repository
2. Create a merge request from this feature branch to main
3. Get approval for the merge request
4. Create multiple commits on the main branch and push these commits sequentially to make the merge request out of date (see script below)
5. While the commits are being pushed, refresh the merge request page to confirm it shows as "behind the target branch"
6. Still while the commits are being pushed, select "Rebase without pipeline"
7. Observe that the approval has been reset
8. Note that the system displays a message indicating approvals were reset "by pushing to the branch"

I used this bash script to generate and push the commits:

#!/bin/bash

# Change to main branch
git checkout main

# Create and push 5 commits with deliberate timing
for i in {1..5}; do
  echo "Clean auto change $i for race test" > main-clean-$i.txt
  git add main-clean-$i.txt
  git commit -m "Clean auto change $i for race test"
  git push origin main
  echo "Pushed commit $i"
  sleep 1  # pause between pushes
done

echo "All commits pushed!"

Example Project

The issue can be reproduced on gitlab.com using any project with merge requests and approvals enabled.

What is the current bug behavior?

When rebasing a merge request using the UI "Rebase without pipeline" button while commits are being pushed to the target branch, approvals are incorrectly reset. The system erroneously identifies the UI-triggered rebase as an external push.

What is the expected correct behavior?

Approvals should not be reset when rebasing a merge request using the UI "Rebase without pipeline" button, as this is an intentional user action rather than an external push that requires re-approval.

Relevant logs and/or screenshots

Race condition occurs in the following code paths:

1. UI Rebase Initiation in app/controllers/projects/merge_requests_controller.rb:

def rebase
  @merge_request
    .rebase_async(current_user.id, skip_ci: Gitlab::Utils.to_boolean(merge_params[:skip_ci], default: false))
  head :ok
end

2. Repository Rebase Implementation in app/models/repository.rb:

def rebase(user, merge_request, skip_ci: false)
  # ...
  merge_request.update!(rebase_commit_sha: commit_id, merge_error: nil)
  # ...
rescue StandardError => e
  merge_request.update!(rebase_commit_sha: nil)
  raise e
end

3. Approval Reset Check in ee/app/services/ee/merge_requests/refresh_service.rb:

def reset_approvals_for_merge_requests(ref, newrev)
  # Add a flag that prevents unverified changes from getting through
  merge_requests_for(push.branch_name, mr_states: [:opened, :closed]).each do |mr|
    if reset_approvals?(mr, newrev)
      mr.approval_state.temporarily_unapprove!
    end
  end

  # We need to make sure the code owner approval rules have all been synced
  # first, so we delay for 10s.
  MergeRequestResetApprovalsWorker.perform_in(10.seconds, project.id, current_user.id, ref, newrev)
end

Screenshot of approvals being reset after rebase:

Possible fixes

Note: This information comes from a Duo Workflow analysis

Several possible solutions:

1. Add an explicit flag for UI-triggered rebases:

# When initiating a UI rebase, set a flag
merge_request.update!(is_ui_rebase: true)

# In the approval reset check
def reset_approvals?(merge_request, newrev)
  return false if merge_request.is_ui_rebase?
  
  # Original check
  !merge_request.merge_train_car && (
    delete_approvals?(merge_request) || 
    merge_request.target_project.project_setting.selective_code_owner_removals
  )
end

2. Improve the existing mechanism:

def reset_approvals?(merge_request, newrev)
  # Don't reset if this is a rebase
  return false if merge_request.rebase_in_progress? || merge_request.rebase_commit_sha == newrev
  
  # Original check
  !merge_request.merge_train_car && (
    delete_approvals?(merge_request) || 
    merge_request.target_project.project_setting.selective_code_owner_removals
  )
end

3. Update record locking to ensure the worker operates on an up-to-date record:

def execute(ref, newrev, ...)
  merge_requests.each do |merge_request|
    merge_request.reload # Ensure we have latest rebase_commit_sha
    # Rest of the code
  end
end

The investigations indicate that the issue also involves MergeRequests::ReloadMergeHeadDiffService where the race condition occurs with SyncCodeOwnerApprovalRules. Long-term solutions would involve either:

1. Extracting Selective Code Owners (SCO) into its own async worker with a delay
2. Refining SCO by abstracting the filtering logic and integrating it into ::MergeRequests::SyncCodeOwnerApprovalRules