Reconcile silently-merged MRs instead of aborting the train

What does this MR do and why?

Fixes #598924. Fast-forward merge trains (merge_method: ff / rebase_merge) use the FromTrainRef merge strategy, which fast-forwards the commit onto the target branch; MergeRequests::PostMergeService then finalises the merge (marks it merged, sets merged_at, fires hooks/notifications, closes issues, deletes the source branch).

If a transient error (e.g. a statement timeout or a Sidekiq shutdown) interrupted that post-merge, MergeTrains::RefreshMergeRequestService's broad rescue StandardError treated it like any other failure and called abort, ejecting the MR from the train and destroying the merge train car. But the commit had already landed on the target branch, so the MR was left open with merged_at: null — inconsistent with git. Re-adding it to the train then reported "no changes to merge after rebasing onto the target branch", and required manual intervention.

RefreshMergeRequestService#merge! now rescues an interrupted merge and reconciles it instead of aborting, covering three windows:

  1. Interrupted after ff_merge landed the commit, before the post-merge finished → run the full post-merge follow-through via MergeRequests::PostMergeService (idempotent through its own merged? guard) plus source-branch deletion. This sets merged_at, fires hooks/notifications and closes issues — a real merge, not a state flip. Reconciliation only runs when the landed commit is actually reachable from the target branch head (mirrors MergeRequest#squash_commit_reachable_from_target_branch?), so a stale sha from a prior aborted attempt cannot qualify. For the FromTrainRef strategy the landed commit is the current train ref sha; other strategies use the recorded merged_commit_sha. That landed sha is also persisted as merged_commit_sha before the post-merge (as the manual-merge refresh path does), since MergeService may not have recorded it before the interruption — otherwise consumers fall back to diff_head_sha, the un-rebased source tip.
  2. Interrupted after the MR was already marked merged (a later post-merge step timed out) → finish the car (mirrors handle_merged_stuck_car); never abort a merged MR.
  3. merge!'s own ProcessError (a soft merge failure) is re-raised so it aborts as before rather than being reconciled.

If the reconciliation itself keeps failing transiently, a ReconcileError makes the worker crash and retry without aborting, so the car is preserved and finalised on a later run.

Both reconcile paths — the immediate one here and the stuck-car unstick path handle_silently_merged_stuck_car — now share finalize_silently_merged_merge!, so a reconciled merge always runs the same full post-merge follow-through. This also closes #627553.

How to set up and validate locally

Setup

  • Configure a GDK project with merge_method: ff in its merge request settings.
  • Enable merge pipelines and merge trains on the project.
  • Ensure an active EE license is loaded.
  • Create an open merge request from a feature branch into the project's default branch (one or two users acting as author and maintainer is sufficient).

Validate Bug A: Reconcile interrupted post-merge

  1. In a rails console session, monkey-patch MergeRequests::PostMergeService#execute to raise an error after calling the original implementation:

    original_execute = MergeRequests::PostMergeService.instance_method(:execute)
    error_raised = false
    MergeRequests::PostMergeService.define_method(:execute) do
      result = original_execute.bind(self).call
      unless error_raised
        error_raised = true
        raise StandardError, "Simulated post-merge timeout"
      end
      result
    end

    Alternatively, add a temporary raise statement in app/services/merge_requests/post_merge_service.rb immediately after the line calling mark_as_merged, then revert it after testing.

  2. Add the merge request to the merge train via the UI or API.

  3. Allow MergeTrains::RefreshMergeRequestWorker to run (or trigger it manually). The ff_merge operation pushes the commit to the target branch, then the injected error fires during post-merge finalization.

  4. Expected behavior before this fix: The MR is ejected from the train, the merge train car is destroyed, the MR remains open with merged_at: null, and re-adding it reports "no changes to merge after rebasing onto the target branch".

  5. Expected behavior with this fix: The MR transitions to merged with merged_at set, and the car is finished. Manual re-addition is not required. A before/after screenshot of the MR widget is helpful here.

Validate Bug B: Finish stuck cars with post-merge follow-through

  1. Create a merge train car in the merging state with updated_at set earlier than the stuck-car timeout threshold, while the underlying MR is already merged in the repository.

  2. Trigger the refresh by allowing MergeTrains::RefreshMergeRequestWorker to run, or by calling directly in rails console:

    MergeTrains::RefreshMergeRequestService.new(project, user).execute(merge_request)
  3. Expected result: The car finishes and the full post-merge follow-through runs, including source branch deletion, closing of related issues, and sending notifications. The fix ensures these side effects occur instead of a bare state-column flip.

Automated test coverage: bundle exec rspec ee/spec/services/merge_trains/refresh_merge_request_service_spec.rb

Known limitations

  • The stuck-car unstick path (handle_silently_merged_stuck_car) can still abort before the MR is marked merged. If PostMergeService fails before it sets the MR to merged (for example, a timeout at that exact point), the code falls through to handle_locked_stuck_car, which unlocks the MR and aborts, destroying the car. This is existing behavior: the new merged? guard only covers the case where the MR is already merged, and the fallback exists to avoid an infinite retry loop. This narrow window is not widened by this MR.
  • Partial post-merge side effects are not retryable. PostMergeService#execute starts with return if merge_request.merged?, so its idempotency means "do nothing if already merged," not "resume where it left off." If it fails partway through, after marking the MR merged but before completing steps such as closing issues, sending notifications, or firing webhooks, those remaining steps are lost and cannot be retried, since the guard short-circuits on subsequent calls. This affects the normal merge path as well, not just merge trains, and is tracked separately in #628217.

Closes #598924 Closes #627553

Edited by Daniel Prause

Merge request reports

Loading
Loading