Allow enabling auto-merge before the merge request diff exists

Marc Shawrequested to merge
fix/auto-merge-skip-commits-check-while-preparing into master

glab mr create --auto-merge creates the MR and immediately asks the merge endpoint to set auto-merge. On 19.4.1 that request returns 422 "Branch cannot be merged", because the MR is still preparing and has no diff yet.

This MR lets the when-checks-pass auto-merge strategies skip the commits check while the MR is preparing. The request then returns 200 and auto-merge is set. The merge itself still waits for the full mergeability checks, so nothing can merge early.

The change is behind the auto_merge_skip_commits_check_while_preparing flag (gitlab_com_derisk, default off, actor: project). With the flag off, behavior is unchanged.

Detailed context for AI agents

Root cause chain

  1. glab mr create --auto-merge sends POST /projects/:id/merge_requests, then right away sends PUT /projects/:id/merge_requests/:iid/merge with auto_merge: true.
  2. MergeRequests::CreateService defers creating the merge request diff to NewMergeRequestWorker (via skip_ensure_merge_request_diff). Until Sidekiq runs that worker, the MR is in preparing state with no diff.
  3. With no diff, has_no_commits? is true, so MergeRequests::Mergeability::CheckCommitsStatusService fails. That check was not skippable.
  4. Because the check fails, AutoMergeService#preferred_strategy returns nil.
  5. There is no pipeline yet, so pipeline_allows_merge? in lib/api/merge_requests.rb is true. The endpoint falls through to execute_immediate_merge!.
  6. mergeable? is false there, so the endpoint renders 422 "Branch cannot be merged".
  7. The same PUT a moment later, after the worker has run, returns 200 and enables auto-merge.

Earlier versions returned 405 for a later window (diff present, conflict check still checking). That was fixed by !245582 (merged) (flag auto_merge_skip_conflict_check, since removed). This MR is the next check in the same chain.

The fix

  • MergeRequest#skipped_auto_merge_checks now includes skip_commits_check.
  • It is true only when all of these hold: the strategy is merge_when_checks_pass or add_to_merge_train_when_checks_pass, the MR is preparing?, and the flag is enabled for the project.
  • CheckCommitsStatusService#skip? now honours params[:skip_commits_check].
  • Flag on: the PUT returns 200 and sets auto-merge during the preparing window.
  • Flag off: behavior is unchanged (422).

Why there is no new race

The skip only affects the availability check, meaning whether auto-merge may be enabled. It does not affect the merge.

  • AutoMerge::MergeWhenChecksPassService#process re-runs mergeable?(use_cache: false) with no skips before it merges.
  • The conflict check fails until the worker's mergeability check moves the MR to can_be_merged.
  • AfterCreateService registers pipeline creation (create_pipeline_for) before check_mergeability. So the CI check holds the merge until the pipeline is settled.
  • This is the same window !245582 (merged) already opened, only reached earlier.
  • A throwaway probe spec ran AutoMergeProcessWorker in three windows: no diff; diff present but pipeline not yet registered; prepared with the pipeline still being created. The MR stayed open in all three.

Alternatives rejected

  • Fix it in glab by polling until detailed_merge_status leaves preparing before the PUT. Rejected as the primary fix, because the API should accept a valid request. glab could still do this to support older instances.

Known side effects

  • Auto-merge can now be enabled on a branch that turns out to have no commits. It stays enabled, with the commits check failing, until someone cancels it.
  • Clients that pass sha still get 409 in the no-diff window, because diff_head_sha is nil. glab does not pass sha, so it is not affected.

Flag and rollout state

  • Flag: auto_merge_skip_commits_check_while_preparing, type gitlab_com_derisk, default off, actor: project. Definition file: config/feature_flags/gitlab_com_derisk/auto_merge_skip_commits_check_while_preparing.yml.
  • Rollout issue: not created yet. It will be linked here once it exists.
  • Milestone: 19.5.
  • No changelog entry, because the change is behind a default-off flag.

Tests

  • spec/models/merge_request_spec.rb: #skipped_auto_merge_checks while preparing, and with the flag off.
  • ee/spec/models/merge_request_spec.rb: the exclusion list.
  • spec/services/merge_requests/mergeability/check_commits_status_service_spec.rb: #skip?.
  • spec/requests/api/merge_requests_spec.rb: new context "when the MR is still preparing and its diff has not been created yet". Flag on returns 200; flag off returns 422.
  • Result: 14 examples, 0 failures locally. Rubocop is clean. An end-to-end request spec (POST then immediate PUT with Sidekiq faked) returns 200 with the fix and 422 without it.

How to validate locally

  1. Use a project that has a repository, and pause Sidekiq (or plan to run the two requests back to back in a script).
  2. Run POST /api/v4/projects/:id/merge_requests to create an MR.
  3. Immediately run PUT /api/v4/projects/:id/merge_requests/:iid/merge with auto_merge=true.
  4. With the flag off, expect 422.
  5. Run Feature.enable(:auto_merge_skip_commits_check_while_preparing, project) and repeat steps 2 and 3 with a new MR. Expect 200 and auto_merge_enabled true.
  6. Alternatively, run glab mr create --auto-merge against the instance with the flag off and then on.

Out of scope

  • glab prints "Failed to create merge request. Created recovery file" even though the MR was created. That is a separate glab bug.
  • Making glab wait for preparing to finish for older instances.
  • Handling the 409 for clients that pass sha in the no-diff window.

References

Merge request reports