Fix MWCP to respect skipped pipelines setting

Marc Shawrequested to merge
fix/mwcp-respect-skipped-pipeline-setting into master

What does this MR do and why?

Fixes auto-merge (MWCP and AMTWCP) to correctly handle skipped pipelines when the project has allow_merge_on_skipped_pipeline enabled.

The bug

The process method in MergeWhenChecksPassService had an early guard:

return if merge_request.has_ci_enabled? && !merge_request.diff_head_pipeline_success?

This guard rejected skipped pipelines before mergeable? was ever called, even though CheckCiStatusService (invoked by mergeable?) already correctly handles skipped pipelines by checking allow_merge_on_skipped_pipeline?.

The same issue existed in the EE AddToMergeTrainWhenChecksPassService.

The fix

Remove the redundant early CI guard entirely from both MergeWhenChecksPassService and AddToMergeTrainWhenChecksPassService, and let mergeable? be the single source of truth for all CI status checks. This eliminates the duplicated logic that caused the bug and prevents similar issues in the future.

The change is gated behind the mwcp_skip_ci_guard feature flag (disabled by default) for safe, incremental rollout on GitLab.com.

The mergeable? call already handles all CI pipeline states correctly via CheckCiStatusService:

  • Pipeline succeeded → merge proceeds
  • Pipeline failed → merge blocked
  • Pipeline running → merge blocked
  • Pipeline skipped + allow_merge_on_skipped_pipeline enabled → merge proceeds
  • Pipeline skipped + allow_merge_on_skipped_pipeline disabled → merge blocked
  • No pipeline + CI enabled → merge blocked
  • No CI at all → merge proceeds

Feature flag

This change is behind the mwcp_skip_ci_guard feature flag:

  • Name: mwcp_skip_ci_guard
  • Type: gitlab_com_derisk
  • Default: disabled
  • Rollout issue: #594568

When enabled: The redundant early CI guard is removed, allowing mergeable? to be the single source of truth. This fixes skipped pipeline handling.

When disabled (default): The old behavior is preserved with the early CI guard in place.

Test changes

Replaced implementation-coupled unit tests (that stubbed has_ci_enabled? and diff_head_pipeline_success?) with full-stack integration tests that exercise the real process → mergeable? → CheckCiStatusService path across all 7 CI pipeline state scenarios listed above. Added feature flag disabled contexts to verify the old behavior is preserved when the flag is off.

How to set up and validate locally

  1. Create a project with only_allow_merge_if_pipeline_succeeds and allow_merge_on_skipped_pipeline both enabled
  2. Create an MR with a CI config where all jobs are when: manual + allow_failure: true (produces a skipped pipeline)
  3. Enable the feature flag: Feature.enable(:mwcp_skip_ci_guard)
  4. Set auto-merge (MWCP) on the MR
  5. Observe that auto-merge now correctly proceeds when the skipped pipeline completes

Validated on local GDK:

On master (before fix):

Pipeline status: skipped
has_ci_enabled?: true
diff_head_pipeline_success?: false
mergeable?: true
BUG CONFIRMED: Auto-merge did NOT proceed with skipped pipeline

On fix branch (after fix, flag enabled):

Pipeline status: skipped
has_ci_enabled?: true
diff_head_pipeline_success?: false
mergeable?: true
SUCCESS: Auto-merge proceeded with skipped pipeline!

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist.

Edited by Marc Shaw

Merge request reports