Detect rebase-to-no-op in CreateRefService
What does this MR do and why?
Closes part of https://gitlab.com/gitlab-org/gitlab/-/work_items/598820 — Site 1 of three (root-cause detection).
When a project has automatic_rebase_enabled=true and a fast-forward / semi-linear merge method, MergeRequests::CreateRefService#maybe_rebase! calls repository.rebase_to_ref. If the source contains no unique changes against the target (e.g. the MR was branched off another MR whose changes are now on target), the rebase output equals first_parent_sha — the source contributes nothing.
Previously this was undetected. MergeService#commit (app/services/merge_requests/merge_service.rb:94-100) then recorded the MR as merged with:
merged_commit_sha = current target tip(inherited from another MR)merge_commit_sha = nil(stripped via.compact, so the column held its prior nil value)
With force_remove_source_branch=true (the common configuration), DeleteSourceBranchWorker then deleted the source branch and the MR's unique commits became unreachable. This is data loss, not just a UX bug.
This MR adds a guard in maybe_rebase!: if the rebase output equals first_parent_sha, raise CreateRefError. MergeService#try_merge converts that into a user-visible MergeError, leaving the MR in opened state with merge_error populated. No DB lie, no source-branch deletion, no orphaned commits.
Customer evidence
Two customer reports on the auto-rebase rollout issue #524048:
- hij1nx on 18.11 — provided rails-console output showing MR B with
merge_commit_sha = nilandmerged_commit_shaequal to MR A'smerge_commit_sha. - joshk0 on 18.10 — "+1, similar experience of a disappearing commit."
Reproduction
Repro script tmp/issue-524048-toggle-collapse.rb produces all three of the customer's DB signatures (A, B, C) end-to-end through MergeService.execute on local GDK. Before this MR, B's signature reproduces (state=merged, mcs=nil, mcsd=A_mcs). After this MR, B stays opened (state=opened, mcs=nil, mcsd=nil), and the script prints B signature: false / ALL THREE: false.
Site 1 of three
The issue documents three independent fix sites, each defending a different layer. This MR implements Site 1 only — the earliest, most root-cause-targeted guard. Sites 2 and 3 will follow as defense-in-depth:
- Site 1 (this MR):
CreateRefService#maybe_rebase!raises when rebase collapses tofirst_parent_sha. Closes the data-loss path. - Site 2 (planned):
from_source_branch.rb#fast_forward!verifies the target ref actually advanced (mirrorsfrom_train_ref.rb:48-50). - Site 3 (planned):
MergeService#commitdrops.compactso nils get written truthfully.
Site 1 alone is sufficient to prevent the data loss. Sites 2 and 3 add belt-and-braces protection and schema honesty.
Backports
To be requested for 18.10 and 18.11 stable once this lands on master.
Test plan
- New spec:
spec/services/merge_requests/create_ref_service_spec.rb— "when the source contains no unique changes against the target" returns:error. - Full file: 22 examples pass (was 21 before).
- Related:
ee/spec/services/ee/merge_requests/create_ref_service_spec.rb(4 examples),spec/services/merge_requests/merge_strategies/from_source_branch_spec.rb(18 examples) — all green. - E2E: re-running
tmp/issue-524048-toggle-collapse.rbagainst the fixed code, B no longer matches the bug signature.
MR acceptance checklist
- Tests added
- Changelog trailer present (
Changelog: fixed) - Backport MRs for 18.10 and 18.11 (follow-up)