Fix Accept MR API bypassing merge trains when pipeline succeeds

Marc Shawrequested to merge
ms/fix-merge-api-train-bypass into master

Summary

Fixes the Accept MR API (PUT /projects/:id/merge_requests/:iid/merge) silently bypassing merge trains on train-enabled projects when calling with auto_merge=true and the head pipeline had already succeeded.

Why

On a merge-train project, MergeRequest#default_auto_merge_strategy is merge_when_checks_pass, but the strategies actually available are merge_train / add_to_merge_train_when_checks_pass. The endpoint asked "is the default strategy available?" and — finding it wasn't — fell through to MergeService.execute directly, bypassing the train entirely.

What changes (behind fix_merge_api_train_bypass)

  • auto_merge=true: pick AutoMergeService#preferred_strategy(mr) (top of available_strategies) instead of insisting on default_auto_merge_strategy. On train-enabled projects this routes to the merge-train strategy; on non-train projects it remains merge_when_checks_pass.
  • auto_merge=false: unchanged — immediate merge on a mergeable MR. On train-enabled projects, any running train is rebuilt by MergeTrains::CheckStatusService on the resulting target-branch push to preserve its fast-forward invariant. skip_merge_train=true still opts into the restartless variant (EE::MergeRequests::MergeService#skipping_active_merge_train?), which records a skip-merged history car and leaves the train running.

The legacy auto_merge=true code path is kept verbatim behind the FF for instant rollback. Once the rollout is complete the legacy helper and the FF can be removed in a follow-up.

Out of scope

Response-shape changes (returning train-car details from this endpoint) — that's a follow-up enhancement to discuss separately.

Related

Testing instructions

1. Set up a merge-train project in GDK

In a Rails console (gdk rails console):

user    = User.find_by(username: 'root')        # or your dev user
project = Project.find_by_full_path('flightjs/Flight') # any EE project; pick yours

project.ci_cd_settings.update!(
  merge_pipelines_enabled:        true,
  merge_trains_enabled:           true,
  merge_trains_skip_train_allowed: true
)

AutoMergeService.new(project, user).all_strategies_ordered_by_preference
#  => ["merge_train", "add_to_merge_train_when_checks_pass", "merge_when_checks_pass"]

Your GDK needs an Ultimate license for merge_pipelines / merge_trains to be available.

2. Create an MR with a green merge-request pipeline

Push a branch with any small change, open an MR targeting master. Wait for the merge-request pipeline to go green. Sanity-check:

mr = project.merge_requests.find_by(iid: <MR_IID>)
mr.diff_head_pipeline_success?   # => true
mr.mergeable?                    # => true

3. Run the scenarios

export TOKEN=<your PAT with api scope>
export PROJECT_ID=<numeric project id>
export MR_IID=<mr iid>
export BASE="http://gdk.test:3000/api/v4"

A. auto_merge=true + FF on → should add to merge train (the bug fix)

Feature.enable(:fix_merge_api_train_bypass, project)
curl -sS --request PUT \
  --header "PRIVATE-TOKEN: $TOKEN" \
  "$BASE/projects/$PROJECT_ID/merge_requests/$MR_IID/merge?auto_merge=true" \
  | jq '{state, merge_when_pipeline_succeeds, merge_user: .merge_user.username}'

Expected:

  • HTTP 200
  • state: "opened" (not merged)
  • MergeTrains::Car.where(merge_request_id: mr.id).any? → true
  • mr.reload.auto_merge_strategy → "merge_train"

B. auto_merge=false + FF on → immediate merge, train rebuilt

Re-prepare a mergeable MR, then:

curl -sS --request PUT \
  --header "PRIVATE-TOKEN: $TOKEN" \
  "$BASE/projects/$PROJECT_ID/merge_requests/$MR_IID/merge" \
  | jq '{state}'

Expected:

  • HTTP 200
  • state: "merged"
  • No skip-merged MergeTrains::Car row inserted
  • Any running train is rebuilt by MergeTrains::CheckStatusService on the target-branch push

C. skip_merge_train=true + FF on (project allows restartless skip) → restartless immediate merge

Re-prepare a mergeable MR, then:

curl -sS --request PUT \
  --header "PRIVATE-TOKEN: $TOKEN" \
  "$BASE/projects/$PROJECT_ID/merge_requests/$MR_IID/merge?skip_merge_train=true" \
  | jq '{state}'

Expected:

  • HTTP 200
  • state: "merged"
  • A skip-merged MergeTrains::Car row is inserted by EE::MergeRequests::MergeService#after_merge

D. FF off → legacy buggy behavior preserved (rollback safety)

Feature.disable(:fix_merge_api_train_bypass)

Re-prepare a mergeable MR. Run scenario A (auto_merge=true).

Expected (the bug, intentionally preserved behind the flag):

  • HTTP 200
  • state: "merged" (direct merge, bypassing the train)

4. Non-train project sanity check

On a project that does not have merge trains enabled, with the FF on:

  • auto_merge=true on an MR whose pipeline is still running → MR set to merge_when_checks_pass, 200, unchanged from today.
  • auto_merge=false on a mergeable MR → direct merge, 200, unchanged from today.

Test plan

  • Pipeline green
  • Scenario A (auto_merge=true + FF on) → MR added to train, not merged
  • Scenario B (auto_merge=false + FF on) → immediate merge, running train rebuilt
  • Scenario C (skip_merge_train=true + FF on, restartless skip allowed) → immediate merge, skip-merged car inserted
  • Scenario D (FF off) → legacy behavior preserved (regression guard)
  • Non-train project, FF on → unchanged behavior
Edited by Marc Shaw

Merge request reports