Skip to content

Commit 91f00d4

Browse files
committed
feat(runway): move a change's head branch before pushing the target
## Summary ### Why? A provider decides whether a change merged while it processes the push to the target branch, comparing the change's recorded head against what that push makes reachable. `MERGE` and `PROMOTE` satisfy that by construction, but `REBASE` and `SQUASH_REBASE` produce new commit objects — so the change's original head appears nowhere in the target's history and the change is recorded as closed after it has, in every meaningful sense, landed. Reachability alone is not enough, and neither is atomicity. The comparison happens at one instant — while the target update is being processed — against the head the provider has recorded *at that moment*. Three orderings were measured against a live GitHub repository, each landing a rebased change whose new commit is provably an ancestor of the target afterwards: | Ordering | Result | | --- | --- | | target pushed, then head branch | closed, not merged | | both refs in one `git push --atomic` | closed, not merged | | head branch pushed, then target | **merged** | Only a separate, earlier push works. The atomic case fails for the same reason as the post-push case: the provider still evaluates the target update against the head it had recorded before the transaction. ### What? `Params.UpdateHeadBranch` moves each change's head branch to the commit that change became — its last replayed commit under `REBASE`, its single squashed commit under `SQUASH_REBASE` — as its own push, immediately before the target is pushed. It is provider-neutral. The branch is found by matching the change's pinned head SHA against the remote's branch tips, never by parsing a change number or calling an API, so the same mechanism serves a GitHub pull request, a GitLab merge request, or a bare branch. Three cases are declined rather than guessed: no matching branch (the ordinary case for a fork, whose branch lives in another repository), several matching branches, and the target branch itself. None of the three is a failure; those changes land normally. A branch that cannot be moved fails the merge, before the target is pushed. Landing a change while knowing its head could not be moved produces exactly the half-merged state the option exists to prevent, so the merge stops rather than completing into it. Each push carries a lease against the SHA the URI pinned, so an author pushing in the window between reading the remote and updating it fails the lease instead of losing work — and now fails the merge with it. Because the head branch now moves before the point of no return, an attempt that moves it and then loses the target push leaves it on a commit that never landed. Such a branch no longer answers to the pinned SHA, so a retry could not find it by matching tips and would strand it there. A tracker carries the resolved branch and the value the next lease must name across attempts, so the retry moves it on to the commit that did land. Off by default: moving a branch the merger was not asked to move is a surprise unless a deployment opted in. ## Test Plan ✅ `bazel test //runway/extension/merger/git:go_default_test` — 14 cases against a real bare repository covering both rewriting strategies, stacks, fork changes, ambiguous matches, the stale lease, and that `MERGE`/`PROMOTE`/dry-run leave branches untouched. Three are new to this ordering: the head branch has moved even when the target push is then rejected, a branch already moved by a failed attempt is moved on by the retry, and a head branch that cannot be pushed fails the land with the target untouched. Each was confirmed to fail against a deliberately reverted implementation, not merely to pass. ✅ The pre-receive race hook now contends only on the target ref, so it simulates target contention rather than rejecting the head-branch pushes that precede it. Existing retry tests are unaffected — they run with the flag off. ✅ Ordering verified end-to-end against a live GitHub repository, per the table above.
1 parent ec27110 commit 91f00d4

6 files changed

Lines changed: 730 additions & 58 deletions

File tree

runway/extension/merger/git/BUILD.bazel

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ go_library(
66
"author.go",
77
"changeref.go",
88
"git_merger.go",
9+
"headbranch.go",
910
"objects.go",
1011
],
1112
importpath = "github.com/uber/submitqueue/runway/extension/merger/git",
@@ -25,7 +26,10 @@ go_library(
2526

2627
go_test(
2728
name = "go_default_test",
28-
srcs = ["git_merger_test.go"],
29+
srcs = [
30+
"git_merger_test.go",
31+
"headbranch_test.go",
32+
],
2933
data = [
3034
"@git",
3135
"@git//:git_receive_pack",

runway/extension/merger/git/README.md

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,23 @@ Redelivery is safe: once imported, the source head is contained in the target, s
7979

8080
`Merge` commits and reports outputs; `CheckMergeability` runs the identical apply but never pushes, then resets the checkout to discard the local commits and reports empty outputs. A multi-step check commits its intermediate steps locally so it sees the same conflict surface a real merge would.
8181

82-
For a committing merge nothing reaches the remote until the final push (a `PROMOTE` is itself a single atomic fast-forward ref update). A step that fails to apply aborts its in-progress git operation and returns without pushing. If the push fails because the remote tip moved between reset and push, the whole reset/apply/push cycle is retried up to a bounded number of attempts; detection re-fetches the tip and compares it to the SHA the cycle was based on.
82+
For a committing merge nothing reaches the remote until every step has applied cleanly (a `PROMOTE` is itself a single atomic fast-forward ref update). A step that fails to apply aborts its in-progress git operation and returns without pushing. With head-branch updates enabled a merge writes two things rather than one — the head branches first, then the target — and only the target's push is the point of no return. If the push fails because the remote tip moved between reset and push, the whole reset/apply/push cycle is retried up to a bounded number of attempts; detection re-fetches the tip and compares it to the SHA the cycle was based on.
83+
84+
## Head branches
85+
86+
A provider decides whether a change merged while it processes the push to the target branch, comparing the change's recorded head against what that push makes reachable. `MERGE` and `PROMOTE` satisfy that on their own — the first keeps the change's head reachable through second-parent history, the second fast-forwards the target to it. The picking strategies do not: `REBASE` and `SQUASH_REBASE` produce new commits, so the change's original head appears nowhere in the target's history and the change is recorded as closed after it has, in every meaningful sense, landed.
87+
88+
Enabling head-branch updates closes that gap. Before the target is pushed, each change's head branch is moved to the commit that change became — its last replayed commit under `REBASE`, its single squashed commit under `SQUASH_REBASE`. The provider records that new head, and when the target push arrives moments later it finds exactly that commit reachable, so it marks the change merged. Nothing here knows what a pull request is: the branch is found by matching the change's pinned head SHA against the remote's branch tips, so the same mechanism serves a GitHub pull request, a GitLab merge request, or a bare branch.
89+
90+
**The ordering is the mechanism.** Moving the head branch *after* the target has been pushed leaves the provider comparing against the pre-merge head at the only moment it looks, and it records the change closed rather than merged — even though the branch ends up on a commit that is demonstrably in the target. Doing both in a single atomic push behaves the same way, since the provider still evaluates the target update against the head it had recorded beforehand. Only a separate, earlier push works.
91+
92+
Three cases are declined rather than guessed at. A change whose head matches **no branch** on this remote is normally one proposed from a fork, whose branch lives in another repository and is not this merger's to move — such a change lands normally. A head matching **several branches** is ambiguous, and the URI does not say which one the change was proposed from, so rewriting a guess risks clobbering an unrelated branch. The **target branch itself** is never a candidate, so a change whose head coincides with the target tip cannot make the merger rewrite the branch it just landed on.
93+
94+
Each push carries a lease against the SHA the change's URI pinned, so an author who pushes in the window between reading the remote's branches and updating them fails the lease instead of losing their work. A failure to move a branch fails the merge, before the target is pushed: landing a change while knowing its head could not be moved produces exactly the half-merged state the option exists to prevent. The three declined cases above are not failures and do not stop the merge.
95+
96+
A branch a failed attempt already moved is remembered for the next one. Once moved, it no longer sits at the SHA the URI pinned, so a retry could not find it by matching tips and would strand it on a commit that never landed; the attempt's resolved branch and the value the next lease must name are carried forward instead.
97+
98+
Off by default — moving a branch the merger was not asked to move is a surprise unless a deployment opted in.
8399

84100
## Failure classification
85101

0 commit comments

Comments
 (0)