Fix merged manually label when source MR is mid-merge

What does this MR do and why?

Merge requests merged through another merge request no longer get a false "merged manually" system note. The note reads "merged with" the carrying merge request, even when the post-push refresh runs before that merge request finishes merging. Audit and peer-review reporting counts these entries.

Resolves #456426

Root cause

When MR B is merged and its merge commit contains the head of MR A, MergeRequests::RefreshService#post_merge_manually_merged marks A merged and looks up B as the source. The refresh runs async from the push, which happens inside the git merge. MergeService order: Gitaly writes the ref (triggers the refresh), MergeService saves merge_commit_sha, PostMergeService calls mark_as_merged, then create_note writes the state event. The refresh can run anywhere in that window.

The lookup used MergeRequestsFinder with merged_without_event_source: true and state: 'merged', which required B to already have its own sourceless merged event. Otherwise nothing matched and A got "merged manually". The filter was added in commit 74389c2b to avoid linking an MR to others the same refresh had just marked merged.

Reproduction: erisrenee/automerge-test!9 (merged) was merged by erisrenee/automerge-test!10 (merged) (shared merge commit 8dd2c021). The first MR's merged event (15:16:55.506) has source_commit set and source_merge_request_id null. The second MR's merged event was created later, at 15:16:55.807.

Changes
  1. RefreshService#post_merge_manually_merged credits each MR it marks merged to the MR that merged its commit, instead of running MergeRequestsFinder per candidate. It looks up the candidate's own merge commit first, then the pushed commit (@push.newrev), with one cached lookup per SHA. Each lookup takes, in order:

    • a locked MR with in_progress_merge_commit_sha equal to the SHA (new MergeRequest.by_merging_commit_sha scope),
    • a locked or merged MR with merged_commit_sha equal to the SHA and a merged state event with no source (new MergeRequest.with_merged_state_event), which is what a merge in GitLab writes,
    • a locked or merged MR with that merged_commit_sha and no merged state event yet.

    MRs with a merged state event that has a source are never picked (MergeRequest.without_merged_state_event_source).

  2. Before the loop it calls find_caught_up_replica(:project, ...). MergeService sticks the project when it sets and clears in_progress_merge_commit_sha, so a caught-up replica sees one of the two SHAs. With no stored location it reads from the primary.

  3. Gitlab::Checks::MatchingMergeRequest#match? uses the same by_merging_commit_sha scope. The lookup itself stays in RefreshService, because match? decides whether a push to a protected branch is allowed.

  4. The merged_without_event_source finder param, MergeRequest.merged_without_state_event_source and ResourceStateEvent.merged_with_no_event_source are removed (no other callers). ResourceStateEvent.merged_with_event_source replaces the last one.

Why fall back to newrev: a merge in GitLab pushes exactly one commit, the MR's resulting commit. For a fast-forward merge of a stacked MR, BranchPushMergeCommitAnalyzer maps the stacked MR's head to itself, which matches neither SHA on the merging MR. Looking up the candidate's own merge commit first keeps pushes that carry several merges crediting each MR to its own merge.

Why MRs with a sourced merged event are skipped: the refresh sets merged_commit_sha on every MR it marks merged, so an MR attributed by an earlier push would otherwise match as the source. An MR with no merged event ranks last because a failed attempt of the same push can leave a sibling merged without its event; on the retry the merging MR has its event and wins.

Rejected: reordering PostMergeService to write the state event earlier. The push happens before MergeService saves anything, so it narrows the window without closing it, and create_note would record state "locked".

Not handled:

  • If the merge fails after Gitaly moves the target branch but before MergeService saves merged_commit_sha, the merging MR has neither SHA set until UnstickLockedMergeRequestsService runs, so MRs in that push get "merged manually".
  • A retry that runs while the merging MR is still inside PostMergeService (merged, no event yet) can still pick a sibling left merged by the failed attempt.
Database

Two new queries, at most two per push, both on merge_requests with existing indexes. Plans from Database Lab (gitlab-production-main), project gitlab-org/gitlab, real SHAs. No new index.

Locked MR merging the pushed commit:

SELECT "merge_requests".* FROM "merge_requests"
WHERE "merge_requests"."target_project_id" = 278964
  AND "merge_requests"."target_branch" = 'master'
  AND "merge_requests"."state_id" = 4
  AND "merge_requests"."in_progress_merge_commit_sha" = '1cbbbff6d9d19182a4d31c1df434d713c57dcd00'
ORDER BY "merge_requests"."id" ASC LIMIT 1
Limit  (actual time=81.388..81.390 rows=0 loops=1)
  Buffers: shared hit=10 read=43 dirtied=9
  ->  Sort  (actual time=81.387..81.388 rows=0 loops=1)
        Sort Key: merge_requests.id
        ->  Index Scan using idx_mrs_on_target_id_and_created_at_and_state_id on public.merge_requests  (actual time=81.347..81.348 rows=0 loops=1)
              Index Cond: ((merge_requests.target_project_id = 278964) AND (merge_requests.state_id = 4))
              Filter: (((merge_requests.target_branch)::text = 'master'::text) AND ((merge_requests.in_progress_merge_commit_sha)::text = '1cbbbff6d9d19182a4d31c1df434d713c57dcd00'::text))
              Rows Removed by Filter: 4
Time: 87.950 ms (planning 6.487 ms, execution 81.463 ms, I/O read 80.712 ms)

No rows: nothing was locked mid-merge on the clone. The index condition limits the filter to the project's locked MRs (4 here).

Locked or merged MR whose merged commit is the pushed commit (merged_commit_sha is stored as the bytes of the hex string):

SELECT "merge_requests".* FROM "merge_requests"
WHERE "merge_requests"."target_project_id" = 278964
  AND "merge_requests"."target_branch" = 'master'
  AND "merge_requests"."state_id" IN (4, 3)
  AND "merge_requests"."merged_commit_sha" = convert_to('f6a86fc59605bfe7cb8366f710ed253b8500e404', 'UTF8')
  AND (NOT EXISTS (SELECT 1 FROM "resource_state_events" WHERE "resource_state_events"."state" = 3 AND ("resource_state_events"."source_merge_request_id" IS NOT NULL OR "resource_state_events"."source_commit" IS NOT NULL) AND (resource_state_events.merge_request_id = merge_requests.id)))
ORDER BY "merge_requests"."id" ASC LIMIT 1
Limit  (actual time=6.706..6.708 rows=1 loops=1)
  Buffers: shared hit=13 read=4
  ->  Sort  (actual time=6.706..6.707 rows=1 loops=1)
        Sort Key: merge_requests.id
        ->  Nested Loop Anti Join  (actual time=6.673..6.675 rows=1 loops=1)
              ->  Index Scan using index_merge_requests_on_target_project_id_and_merged_commit_sha on public.merge_requests  (actual time=1.569..1.571 rows=1 loops=1)
                    Index Cond: ((merge_requests.target_project_id = 278964) AND (merge_requests.merged_commit_sha = convert_to('f6a86fc59605bfe7cb8366f710ed253b8500e404'::text, 'UTF8'::name)))
                    Filter: ((merge_requests.state_id = ANY ('{4,3}'::integer[])) AND ((merge_requests.target_branch)::text = 'master'::text))
              ->  Index Scan using index_resource_state_events_on_merge_request_id on public.resource_state_events  (actual time=5.098..5.098 rows=0 loops=1)
                    Index Cond: (resource_state_events.merge_request_id = merge_requests.id)
                    Filter: (((source_merge_request_id IS NOT NULL) OR (source_commit IS NOT NULL)) AND (state = 3))
                    Rows Removed by Filter: 1
Time: 13.645 ms (planning 6.864 ms, execution 6.781 ms)

The previous MergeRequestsFinder lookup, including its unscoped by_commit_sha branch (23.2 minutes on gitlab-org/gitlab), no longer runs on this path.

Tests

RefreshService specs cover the merging MR locked before and after its merge SHAs are saved, merged with no state event yet, a fast-forward merge that includes a stacked MR, and an MR marked merged by an earlier push (must not be picked). The fast-forward spec fails with a per-candidate SHA lookup, and the earlier-push spec fails without without_merged_state_event_source. Model specs cover both new scopes.

How to set up and validate locally

  1. Create two merge requests in one project where the second contains the head of the first.
  2. Merge the second merge request in the UI.
  3. Open the first merge request and confirm the system note reads "merged with" the second, not "merged manually".

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.

Edited by Kai Armstrong

Merge request reports

Loading
Loading