Skip to content

Commit f5df112

Browse files
committed
feat(runway): git merger SQUASH_REBASE and MERGE
## Summary ### Why? The git merger landed with REBASE only. `SQUASH_REBASE` and `MERGE` are part of the wire contract SubmitQueue already publishes against, and until they apply here a request naming either is rejected as an invalid request. This adds the two remaining transforming strategies on top of the shared apply machinery. ### What? **SQUASH_REBASE** applies each change exactly like REBASE — replaying every commit it introduces — then collapses what that change produced into a single commit. The squash unit is the change: a change of ten commits becomes one, and a step whose change carries several URIs yields one commit per URI. Those URIs are a stack of pull requests, and squashing them together would erase the per-PR boundary the stack exists to express. Two degenerate cases produce no output rather than an empty commit. A change already present on the target creates no commits, so there is nothing to squash. A change that does create commits whose net tree matches the base would squash to an empty commit, so the intermediates are dropped. Both keep redelivery idempotent. **MERGE** creates a `--no-ff` merge commit per change, which keeps the change's original commits reachable through second-parent history — the property that separates it from the picking strategies, which rewrite those hashes. A change already contained in HEAD is skipped rather than merged again. **Not every failed merge is a conflict.** `applyMerge` previously reported any `git merge` failure as `ErrConflict`, which tells the client its change collides with the target even when nothing collided. Failures are now classified, and the case that matters in practice is an unrelated history. **Importing an unrelated history.** A repository migration arrives as an ordinary change in the target repo whose branch carries the source repo's whole history. Being in the target repo is what makes the commits fetchable; it says nothing about ancestry, and git refuses to merge two graphs with no common ancestor. `AllowUnrelatedHistories` lifts that refusal for a queue that exists to perform such imports. It is off by default because the refusal is a genuine safeguard — with it always on, merging the wrong object silently produces a nonsense result instead of failing. Without the option, the refusal is now reported as an invalid request rather than a conflict. MERGE is the only strategy that can serve a migration: it is the only one that preserves the imported commits' original hashes, and the picking strategies have no range to compute across disjoint graphs, so they reject such a change explicitly and say so. ## Test Plan ✅ `bazel test //runway/extension/merger/git:go_default_test` — passes (58s) New coverage: SQUASH_REBASE collapsing a multi-commit change into one commit while landing all of its content, a two-URI change yielding one squashed commit per URI in application order, SQUASH_REBASE over an already-landed change producing none, MERGE creating one merge commit for a multi-commit change, MERGE skipping a change already an ancestor of the tip, and the MERGE dry-run path. For migration specifically: importing a three-commit unrelated history and asserting every imported commit is reachable **under its original hash**, the merge commit has two parents, and the target keeps its own files; redelivery of the same request being a no-op; the import rejected as an invalid request (explicitly not a conflict) when the option is off, leaving the remote and checkout untouched; and both picking strategies rejecting an unrelated history rather than rewriting it.
1 parent e96766d commit f5df112

3 files changed

Lines changed: 568 additions & 22 deletions

File tree

runway/extension/merger/git/README.md

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,21 @@ Fetching by SHA guarantees the merger applies exactly the commit a URI names —
3838
| Strategy | What it does | Outputs |
3939
|---|---|---|
4040
| `REBASE` | Cherry-picks every commit each change introduces onto the tip, in order. A commit already present on the target is skipped (no output), as is one that was empty to begin with. | one revision per newly-created commit |
41+
| `SQUASH_REBASE` | Applies each change like `REBASE`, then collapses the commits it produced into a single commit (squash unit = the change, not the step). | one revision per change, or none for a change already present |
42+
| `MERGE` | Creates a `--no-ff` merge commit per change, keeping the change's original commits reachable through second-parent history. A commit already contained in the tip is skipped. | the merge-commit revision(s) |
4143
| `DEFAULT` | Resolved to the instance's configured default strategy before any step runs. | per the resolved strategy |
4244

43-
`REBASE` is the only strategy implemented so far. `SQUASH_REBASE`, `MERGE`, and `PROMOTE` are defined by the wire contract but not yet applied here — a step naming one is rejected as an invalid request.
45+
`PROMOTE` is defined by the wire contract but not yet applied here — a step naming it is rejected as an invalid request.
46+
47+
## Importing an unrelated history
48+
49+
A repository migration arrives as an ordinary change in the target repo whose branch carries the source repo's entire history. Living in the target repo is what makes its commits fetchable; it says nothing about ancestry, and the two graphs still share no common ancestor, so git refuses the merge by default.
50+
51+
`MERGE` is the only strategy that can serve this, because it is the only one that leaves the imported commits reachable under their original hashes — the picking strategies would rewrite every one of them, and have no range to compute in the first place, so they reject such a change outright.
52+
53+
The refusal is lifted by a per-instance option rather than always: it is a real safeguard, and without it a merge of the wrong object fails loudly instead of quietly producing a nonsense result. A queue that exists to perform imports turns it on. A refusal that surfaces without the option is reported as an invalid request, not as a conflict — nothing collided.
54+
55+
Redelivery is safe: once imported, the source head is contained in the target, so the change is skipped rather than merged twice.
4456

4557
## Committing, dry-run, atomicity, contention
4658

@@ -56,7 +68,7 @@ The distinction between the last two matters operationally: a commit that is mis
5668

5769
## Runtime and identity
5870

59-
Every git invocation uses the pinned runtime (explicit executable, exec-path, and template dir) and a scrubbed environment: no ambient configuration, no system or global git config, no interactive prompts. Because that leaves no ambient identity, the committer name and email are injected per-invocation, which the commit-creating `REBASE` strategy requires.
71+
Every git invocation uses the pinned runtime (explicit executable, exec-path, and template dir) and a scrubbed environment: no ambient configuration, no system or global git config, no interactive prompts. Because that leaves no ambient identity, the committer name and email are injected per-invocation, which the commit-creating strategies (`REBASE`, `SQUASH_REBASE`, `MERGE`) require.
6072

6173
Scrubbing denies git ambient *configuration* — anything that could change what a merge produces. It deliberately does not deny it the means to reach the remote, so the agent socket, `PATH`, ssh-command, TLS and proxy variables are inherited when set. Without them an SSH remote cannot authenticate and git cannot even exec `ssh`; none of them can influence merge semantics. A deployment needing more can name additional variables on the runtime.
6274

runway/extension/merger/git/git_merger.go

Lines changed: 242 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,15 @@
1919
// Strategy → git operation:
2020
//
2121
// - REBASE: cherry-pick each URI's head commit onto the target tip.
22+
// - SQUASH_REBASE: cherry-pick the step's changes, then collapse them into a
23+
// single commit (squash unit = the step).
24+
// - MERGE: create a --no-ff merge commit per URI, preserving the
25+
// original commit hashes in second-parent history.
2226
// - DEFAULT: resolved to the instance's configured DefaultStrategy
2327
// before any step runs.
2428
//
25-
// REBASE is the only strategy implemented so far. A step naming any other
26-
// strategy is rejected as merger.ErrInvalidRequest until its apply path lands.
29+
// PROMOTE is not implemented yet; a step naming it is rejected as
30+
// merger.ErrInvalidRequest until its apply path lands.
2731
//
2832
// Atomicity: for a committing merge nothing reaches the remote until the final
2933
// push. A step that fails to apply aborts the in-progress git operation and
@@ -77,8 +81,8 @@ const defaultMaxPushAttempts = 10
7781

7882
// Default committer identity used when Params leaves it unset. The scrubbed
7983
// environment (GIT_CONFIG_NOSYSTEM, GIT_CONFIG_GLOBAL=/dev/null) leaves no
80-
// ambient identity, so the commit-creating REBASE strategy needs one supplied
81-
// explicitly.
84+
// ambient identity, so commit-creating strategies (REBASE/SQUASH_REBASE/MERGE)
85+
// need one supplied explicitly.
8286
const (
8387
defaultCommitterName = "SubmitQueue Runway"
8488
defaultCommitterEmail = "runway@submitqueue.invalid"
@@ -112,7 +116,7 @@ type Params struct {
112116
// Target is the destination branch ref on the remote (e.g. "main").
113117
Target string
114118
// DefaultStrategy resolves a step whose strategy is DEFAULT. Must be a
115-
// concrete strategy (currently only REBASE).
119+
// concrete strategy (REBASE, SQUASH_REBASE, or MERGE).
116120
DefaultStrategy mergestrategypb.Strategy
117121
// Runtime is the pinned Git runtime used for every invocation.
118122
Runtime GitRuntime
@@ -129,6 +133,13 @@ type Params struct {
129133
// CheckStaleness enables verifying, before applying, that each change's
130134
// canonical ref still points at the commit its URI names.
131135
CheckStaleness bool
136+
// AllowUnrelatedHistories lets a MERGE step integrate a change that shares
137+
// no ancestry with the target — importing one repository's history into
138+
// another. Off by default: the refusal it lifts is a real safeguard, since
139+
// without it a merge of the wrong object fails loudly instead of quietly
140+
// producing a nonsense result. Enable it on a queue that exists to perform
141+
// such an import.
142+
AllowUnrelatedHistories bool
132143
// CommitterName / CommitterEmail identify the author/committer of
133144
// service-created commits. Defaults are used when empty.
134145
CommitterName string
@@ -150,10 +161,13 @@ type gitMerger struct {
150161
maxPushAttempts int
151162
fetchRefspecs []string
152163
checkStaleness bool
153-
committerName string
154-
committerEmail string
155-
logger *zap.SugaredLogger
156-
metricsScope tally.Scope
164+
165+
// allowUnrelatedHistories permits a MERGE across disjoint history graphs.
166+
allowUnrelatedHistories bool
167+
committerName string
168+
committerEmail string
169+
logger *zap.SugaredLogger
170+
metricsScope tally.Scope
157171

158172
// mu serializes concurrent operations — the underlying checkout cannot be
159173
// safely shared between operations.
@@ -177,7 +191,7 @@ func NewMerger(params Params) (merger.Merger, error) {
177191
return nil, err
178192
}
179193
if !isConcreteStrategy(params.DefaultStrategy) {
180-
return nil, fmt.Errorf("default strategy must be concrete (currently only REBASE), got %v", params.DefaultStrategy)
194+
return nil, fmt.Errorf("default strategy must be concrete (REBASE, SQUASH_REBASE, or MERGE), got %v", params.DefaultStrategy)
181195
}
182196
maxAttempts := params.MaxPushAttempts
183197
if maxAttempts <= 0 {
@@ -200,10 +214,12 @@ func NewMerger(params Params) (merger.Merger, error) {
200214
maxPushAttempts: maxAttempts,
201215
fetchRefspecs: params.FetchRefspecs,
202216
checkStaleness: params.CheckStaleness,
203-
committerName: committerName,
204-
committerEmail: committerEmail,
205-
logger: params.Logger.Named("git_merger"),
206-
metricsScope: params.MetricsScope.SubScope("git_merger"),
217+
218+
allowUnrelatedHistories: params.AllowUnrelatedHistories,
219+
committerName: committerName,
220+
committerEmail: committerEmail,
221+
logger: params.Logger.Named("git_merger"),
222+
metricsScope: params.MetricsScope.SubScope("git_merger"),
207223
}, nil
208224
}
209225

@@ -299,7 +315,8 @@ func (m *gitMerger) resolveAndValidate(req *runwaymq.MergeRequest) ([]resolvedSt
299315
}
300316

301317
// applyTransforming runs the reset/apply/push cycle for the transforming
302-
// strategies, retrying on remote contention when committing. For a dry run it applies the steps locally then discards them.
318+
// strategies (REBASE, SQUASH_REBASE, MERGE), retrying on remote contention when
319+
// committing. For a dry run it applies the steps locally then discards them.
303320
func (m *gitMerger) applyTransforming(ctx context.Context, req *runwaymq.MergeRequest, steps []resolvedStep, commit bool) (*runwaymq.MergeResult, error) {
304321
var lastErr error
305322
for attempt := 1; attempt <= m.maxPushAttempts; attempt++ {
@@ -408,6 +425,10 @@ func (m *gitMerger) applySteps(ctx context.Context, steps []resolvedStep) ([]*ru
408425
switch rs.strategy {
409426
case mergestrategypb.Strategy_REBASE:
410427
outputs, err = m.applyRebase(ctx, rs.step)
428+
case mergestrategypb.Strategy_SQUASH_REBASE:
429+
outputs, err = m.applySquashRebase(ctx, rs.step)
430+
case mergestrategypb.Strategy_MERGE:
431+
outputs, err = m.applyMerge(ctx, rs.step)
411432
default:
412433
// resolveAndValidate rejects anything else; defensive.
413434
return nil, fmt.Errorf("%w: unsupported strategy %v", merger.ErrInvalidRequest, rs.strategy)
@@ -430,6 +451,149 @@ func (m *gitMerger) applyRebase(ctx context.Context, step *runwaymq.MergeStep) (
430451
return toOutputs(picked), nil
431452
}
432453

454+
// applySquashRebase collapses each change in the step into a single commit.
455+
//
456+
// The squash unit is the change, not the step: a change is one pull request,
457+
// and squashing it is what "squash the PR" means. A step whose change carries
458+
// several URIs is a stack of pull requests, and collapsing those into one
459+
// commit would erase the per-PR boundary the stack exists to express. So each
460+
// URI is picked as its own range and squashed on its own, in order, yielding
461+
// one commit — and one output — per change that had anything to contribute.
462+
func (m *gitMerger) applySquashRebase(ctx context.Context, step *runwaymq.MergeStep) ([]*runwaymq.StepOutput, error) {
463+
var outputs []*runwaymq.StepOutput
464+
for _, uri := range step.GetChange().GetUris() {
465+
ref, err := resolveChange(uri)
466+
if err != nil {
467+
return nil, err
468+
}
469+
sha, squashed, err := m.squashChange(ctx, step, ref)
470+
if err != nil {
471+
return nil, err
472+
}
473+
if squashed {
474+
outputs = append(outputs, &runwaymq.StepOutput{Id: sha})
475+
}
476+
}
477+
return outputs, nil
478+
}
479+
480+
// squashChange replays one change and collapses whatever it produced into a
481+
// single commit, reporting squashed=false when it produced nothing worth
482+
// keeping.
483+
//
484+
// Two cases produce nothing. A change already present on the target creates no
485+
// commits at all. A change that does create commits whose net tree matches the
486+
// base — its effect was already there, spread differently — would squash to an
487+
// empty commit, so the intermediates are dropped instead. Both keep redelivery
488+
// idempotent.
489+
func (m *gitMerger) squashChange(ctx context.Context, step *runwaymq.MergeStep, ref changeRef) (string, bool, error) {
490+
preSHA, err := m.headSHA(ctx)
491+
if err != nil {
492+
return "", false, err
493+
}
494+
495+
created, err := m.pickRange(ctx, ref)
496+
if err != nil {
497+
return "", false, err
498+
}
499+
if len(created) == 0 {
500+
return "", false, nil
501+
}
502+
503+
preTree, err := m.commitTreeSHA(ctx, preSHA)
504+
if err != nil {
505+
return "", false, err
506+
}
507+
postTree, err := m.commitTreeSHA(ctx, "HEAD")
508+
if err != nil {
509+
return "", false, err
510+
}
511+
if preTree == postTree {
512+
if _, err := m.run(ctx, nil, "reset", "--hard", preSHA); err != nil {
513+
return "", false, fmt.Errorf("git reset --hard %s after empty squash: %w", preSHA, err)
514+
}
515+
return "", false, nil
516+
}
517+
518+
if _, err := m.run(ctx, nil, "reset", "--soft", preSHA); err != nil {
519+
return "", false, fmt.Errorf("git reset --soft %s: %w", preSHA, err)
520+
}
521+
if _, err := m.run(ctx, nil, "commit", "-m", squashMessage(step, ref)); err != nil {
522+
return "", false, fmt.Errorf("git commit (squash): %w", err)
523+
}
524+
sha, err := m.headSHA(ctx)
525+
if err != nil {
526+
return "", false, err
527+
}
528+
return sha, true, nil
529+
}
530+
531+
// applyMerge creates a --no-ff merge commit for every change in the step,
532+
// keeping the change's original commits reachable through second-parent
533+
// history — the property that distinguishes MERGE from the picking strategies,
534+
// which rewrite those commits. A change already contained in HEAD produces no
535+
// output, which is what makes redelivery idempotent.
536+
func (m *gitMerger) applyMerge(ctx context.Context, step *runwaymq.MergeStep) ([]*runwaymq.StepOutput, error) {
537+
var outputs []*runwaymq.StepOutput
538+
for _, uri := range step.GetChange().GetUris() {
539+
ref, err := resolveChange(uri)
540+
if err != nil {
541+
return nil, err
542+
}
543+
544+
contained, err := m.isAncestor(ctx, ref.SHA, "HEAD")
545+
if err != nil {
546+
return nil, err
547+
}
548+
if contained {
549+
continue
550+
}
551+
552+
args := []string{"merge", "--no-ff", "--no-edit"}
553+
if m.allowUnrelatedHistories {
554+
args = append(args, "--allow-unrelated-histories")
555+
}
556+
out, err := m.runCombined(ctx, nil, append(args, ref.SHA)...)
557+
if err != nil {
558+
// Read the index before aborting clears it; a non-zero exit alone
559+
// does not establish that anything collided.
560+
conflicted := m.hasUnmergedPaths(ctx)
561+
_, _ = m.run(ctx, nil, "merge", "--abort")
562+
return nil, m.classifyMergeFailure(ref, out, conflicted)
563+
}
564+
mergeSHA, err := m.headSHA(ctx)
565+
if err != nil {
566+
return nil, err
567+
}
568+
outputs = append(outputs, &runwaymq.StepOutput{Id: mergeSHA})
569+
}
570+
return outputs, nil
571+
}
572+
573+
// classifyMergeFailure decides what a failed `git merge` actually means, given
574+
// whether the index was left holding conflicted entries.
575+
//
576+
// Not every refusal is a conflict, and reporting one as such tells the client
577+
// its change collides with the target when nothing of the sort happened. Two
578+
// non-conflicts land here: an import of an unrelated history, refused outright
579+
// and fixed by configuration rather than by rebasing, and any other way git
580+
// can exit non-zero — a missing object, an unreadable repository, a killed
581+
// process — which is infrastructure and should be retried, not made terminal.
582+
func (m *gitMerger) classifyMergeFailure(ref changeRef, out []byte, conflicted bool) error {
583+
detail := strings.TrimSpace(string(out))
584+
if strings.Contains(detail, "refusing to merge unrelated histories") {
585+
coremetrics.NamedCounter(m.metricsScope, "merge", "unrelated_histories", 1)
586+
return fmt.Errorf("%w: %s shares no history with the merge target; integrating an imported history requires the merger to allow unrelated histories",
587+
merger.ErrInvalidRequest, ref.Label)
588+
}
589+
if !conflicted {
590+
coremetrics.NamedCounter(m.metricsScope, "merge", "merge_errors", 1)
591+
return fmt.Errorf("git merge %s: %s", ref.SHA, detail)
592+
}
593+
coremetrics.NamedCounter(m.metricsScope, "merge", "merge_conflicts", 1)
594+
return fmt.Errorf("%w: git merge %s: %s", merger.ErrConflict, ref.SHA, detail)
595+
}
596+
433597
// pickStepChanges applies every change in the step, in order, returning the
434598
// SHAs of the commits created on the target (empty for a change whose content
435599
// was already present).
@@ -614,6 +778,52 @@ func (m *gitMerger) refetchTipSHA(ctx context.Context) (string, error) {
614778
return strings.TrimSpace(string(out)), nil
615779
}
616780

781+
// isAncestor reports whether ancestor is an ancestor of (or equal to)
782+
// descendant. `git merge-base --is-ancestor` exits 0 for true, 1 for false;
783+
// any other exit is a real error.
784+
func (m *gitMerger) isAncestor(ctx context.Context, ancestor, descendant string) (bool, error) {
785+
cmd := m.command(ctx, "merge-base", "--is-ancestor", ancestor, descendant)
786+
var stderr bytes.Buffer
787+
cmd.Stderr = &stderr
788+
err := cmd.Run()
789+
if err == nil {
790+
return true, nil
791+
}
792+
var exitErr *exec.ExitError
793+
if errors.As(err, &exitErr) && exitErr.ExitCode() == 1 {
794+
return false, nil
795+
}
796+
return false, fmt.Errorf("git merge-base --is-ancestor %s %s: %w: %s", ancestor, descendant, err, strings.TrimSpace(stderr.String()))
797+
}
798+
799+
// isEmptyHEADCommit returns true when HEAD's tree matches HEAD^'s tree — i.e.
800+
// the most recent commit introduces no changes.
801+
func (m *gitMerger) isEmptyHEADCommit(ctx context.Context) (bool, error) {
802+
headTree, err := m.commitTreeSHA(ctx, "HEAD")
803+
if err != nil {
804+
return false, err
805+
}
806+
parentTree, err := m.commitTreeSHA(ctx, "HEAD^")
807+
if err != nil {
808+
return false, err
809+
}
810+
return headTree == parentTree, nil
811+
}
812+
813+
// commitTreeSHA returns the tree SHA recorded in the commit object at ref.
814+
func (m *gitMerger) commitTreeSHA(ctx context.Context, ref string) (string, error) {
815+
out, err := m.run(ctx, nil, "cat-file", "commit", ref)
816+
if err != nil {
817+
return "", fmt.Errorf("git cat-file commit %s: %w", ref, err)
818+
}
819+
firstLine, _, _ := strings.Cut(string(out), "\n")
820+
const prefix = "tree "
821+
if !strings.HasPrefix(firstLine, prefix) {
822+
return "", fmt.Errorf("git cat-file commit %s: unexpected first line %q", ref, firstLine)
823+
}
824+
return strings.TrimSpace(firstLine[len(prefix):]), nil
825+
}
826+
617827
// push pushes the current HEAD to refs/heads/<target> on the remote.
618828
func (m *gitMerger) push(ctx context.Context) error {
619829
refspec := "HEAD:refs/heads/" + m.target
@@ -746,12 +956,13 @@ func passthroughEnv(extra []string) []string {
746956
}
747957

748958
// isConcreteStrategy reports whether s names a concrete integration strategy
749-
// (i.e. not DEFAULT and not an unknown value). Only REBASE is implemented so
750-
// far; the remaining strategies are rejected as invalid requests until their
751-
// apply paths land.
959+
// (i.e. not DEFAULT and not an unknown value). PROMOTE is not implemented yet
960+
// and is rejected as an invalid request until its apply path lands.
752961
func isConcreteStrategy(s mergestrategypb.Strategy) bool {
753962
switch s {
754-
case mergestrategypb.Strategy_REBASE:
963+
case mergestrategypb.Strategy_REBASE,
964+
mergestrategypb.Strategy_SQUASH_REBASE,
965+
mergestrategypb.Strategy_MERGE:
755966
return true
756967
default:
757968
return false
@@ -766,6 +977,18 @@ func isRedundantCherryPick(out []byte) bool {
766977
strings.Contains(s, "nothing to commit")
767978
}
768979

980+
// squashMessage synthesizes a commit message for one squashed change. The wire
981+
// carries no upstream commit message, so the change is named by its provider
982+
// label — which is why this goes through the resolver rather than one
983+
// provider's fields, so a non-GitHub change is named too instead of being
984+
// silently omitted.
985+
func squashMessage(step *runwaymq.MergeStep, ref changeRef) string {
986+
if step.GetStepId() != "" {
987+
return fmt.Sprintf("squash: %s (%s)", step.GetStepId(), ref.Label)
988+
}
989+
return fmt.Sprintf("squash: %s", ref.Label)
990+
}
991+
769992
// toOutputs wraps commit SHAs as StepOutputs in order.
770993
func toOutputs(shas []string) []*runwaymq.StepOutput {
771994
if len(shas) == 0 {

0 commit comments

Comments
 (0)