Move the conflict resolution gates into ResolveService

Marcel van Remmerdenrequested to merge
refactor/conflicts-resolve-service-gates into master

What does this MR do and why?

The conflicts controller's resolve_conflicts action and the mergeRequestResolveConflicts GraphQL mutation (!256558 (closed)) each checked push access to the source branch, mergeability, and whether the conflicts are resolvable in the UI, tracked the same usage counter, and rescued the same Gitaly and git errors. Keeping two copies of these checks meant they were bound to drift.

This MR moves all of it into MergeRequests::Conflicts::ResolveService#execute, which now returns a ServiceResponse. On failure its reason says which check failed, and the conflicts controller maps that to a status code:

  • no_push_access, mergeability_unchecked, not_resolvable_in_ui: 404, since there is nothing for this user to resolve here.
  • pre_receive: 422, the push was rejected by a hook.
  • gitaly_unavailable: 503.
  • everything else (already_resolved, file_not_in_conflict, incomplete_file, resolution_error): 400.

Centralizing the checks also fixed three gaps:

  • A file whose path was not actually in conflict used to reach Gitaly, which reports it as an internal error. That read as an outage and created a Sentry event for what was really a client mistake. The service now checks the path against the conflict list first and refuses with file_not_in_conflict, "File %{path} is not in conflict.", before calling Gitaly.
  • Right after a push, while the mergeability check is still running, the checks could not tell that case apart from conflicts that genuinely cannot be resolved in the UI, so a caller surfacing the message would have sent the user off to resolve locally. The service now returns a distinct mergeability_unchecked reason with "The mergeability of this merge request is still being checked. Please try again in a moment." The controller keeps answering 404 for it, as before.
  • On master, resolving an already-resolved merge request through the web endpoint answered 404, because the controller's can_be_resolved_in_ui? check ran before the "already resolved" check could apply. The service's checks run in a different order, so this now answers 400 with "The merge conflicts for this merge request have already been resolved." instead.

It also closed two smaller edge cases: a blank commit_message now falls back to the default merge commit message instead of raising ArgumentError (previously a 500); and a sections value that is present but not a hash, such as a string, now fails the same "needs either sections or content" check an empty value already failed, instead of reaching Gitaly and surfacing as a gitaly_unavailable Sentry event.

HandlesGitalyErrors.gitaly_unavailable_message becomes a module method as part of this, so the service can produce the same SaaS and self-managed wording the controller uses, without needing a controller instance.

This is groundwork split off !256558 (closed), so that MR's GraphQL mutation and this controller share one set of gates instead of each keeping its own copy.

References

Screenshots or screen recordings

Not applicable. Backend refactor, no UI change.

How to set up and validate locally

  1. Create a merge request whose branches conflict (the spec fixture branches conflict-resolvable into conflict-start in the test repository work for this).
  2. Resolve the conflicts through the web conflicts UI as before. The only visible change is that resolving an already-resolved merge request now answers 400 with "The merge conflicts for this merge request have already been resolved." instead of a 404.
  3. To see the new refusal, post JSON directly to the resolve_conflicts action (path pattern /:namespace/:project/-/merge_requests/:iid/conflicts/resolve_conflicts) with a file whose new_path is not one of the conflicting files. Expect a 400 with {"message": "File <path> is not in conflict."}.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

  • Specs: spec/services/merge_requests/conflicts/resolve_service_spec.rb, spec/controllers/projects/merge_requests/conflicts_controller_spec.rb, spec/controllers/concerns/handles_gitaly_errors_spec.rb.
  • Generated: translation catalogue (locale/gitlab.pot), four new strings.
  • No database change, no feature flag, no UI change.

🤖 Generated with Claude Code

Edited by Marcel van Remmerden

Merge request reports