Skip to content

Commit 5ae36dd

Browse files
behinddwallsgithub-actions[bot]
authored andcommitted
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 d57b36b commit 5ae36dd

3 files changed

Lines changed: 540 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: 214 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.
@@ -182,7 +196,7 @@ func NewMerger(params Params) (merger.Merger, error) {
182196
return nil, err
183197
}
184198
if !isConcreteStrategy(params.DefaultStrategy) {
185-
return nil, fmt.Errorf("default strategy must be concrete (currently only REBASE), got %v", params.DefaultStrategy)
199+
return nil, fmt.Errorf("default strategy must be concrete (REBASE, SQUASH_REBASE, or MERGE), got %v", params.DefaultStrategy)
186200
}
187201
maxAttempts := params.MaxPushAttempts
188202
if maxAttempts <= 0 {
@@ -205,10 +219,12 @@ func NewMerger(params Params) (merger.Merger, error) {
205219
maxPushAttempts: maxAttempts,
206220
fetchRefspecs: params.FetchRefspecs,
207221
checkStaleness: params.CheckStaleness,
208-
committerName: committerName,
209-
committerEmail: committerEmail,
210-
logger: params.Logger.Named("git_merger"),
211-
metricsScope: params.MetricsScope.SubScope("git_merger"),
222+
223+
allowUnrelatedHistories: params.AllowUnrelatedHistories,
224+
committerName: committerName,
225+
committerEmail: committerEmail,
226+
logger: params.Logger.Named("git_merger"),
227+
metricsScope: params.MetricsScope.SubScope("git_merger"),
212228
}, nil
213229
}
214230

@@ -307,7 +323,8 @@ func (m *gitMerger) resolveAndValidate(req *runwaymq.MergeRequest) ([]resolvedSt
307323
}
308324

309325
// applyTransforming runs the reset/apply/push cycle for the transforming
310-
// strategies, retrying on remote contention when committing. For a dry run it applies the steps locally then discards them.
326+
// strategies (REBASE, SQUASH_REBASE, MERGE), retrying on remote contention when
327+
// committing. For a dry run it applies the steps locally then discards them.
311328
func (m *gitMerger) applyTransforming(ctx context.Context, req *runwaymq.MergeRequest, steps []resolvedStep, commit bool) (*runwaymq.MergeResult, error) {
312329
// Fetch and vet every change the request names before the first attempt, so
313330
// an unusable request fails without having touched the checkout. This sits
@@ -416,6 +433,10 @@ func (m *gitMerger) applySteps(ctx context.Context, steps []resolvedStep) ([]*ru
416433
switch rs.strategy {
417434
case mergestrategypb.Strategy_REBASE:
418435
outputs, err = m.applyRebase(ctx, rs)
436+
case mergestrategypb.Strategy_SQUASH_REBASE:
437+
outputs, err = m.applySquashRebase(ctx, rs)
438+
case mergestrategypb.Strategy_MERGE:
439+
outputs, err = m.applyMerge(ctx, rs)
419440
default:
420441
// resolveAndValidate rejects anything else; defensive.
421442
return nil, fmt.Errorf("%w: unsupported strategy %v", merger.ErrInvalidRequest, rs.strategy)
@@ -438,6 +459,140 @@ func (m *gitMerger) applyRebase(ctx context.Context, rs resolvedStep) ([]*runway
438459
return toOutputs(picked), nil
439460
}
440461

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

776+
// isAncestor reports whether ancestor is an ancestor of (or equal to)
777+
// descendant. `git merge-base --is-ancestor` exits 0 for true, 1 for false;
778+
// any other exit is a real error.
779+
func (m *gitMerger) isAncestor(ctx context.Context, ancestor, descendant string) (bool, error) {
780+
cmd := m.command(ctx, "merge-base", "--is-ancestor", ancestor, descendant)
781+
var stderr bytes.Buffer
782+
cmd.Stderr = &stderr
783+
err := cmd.Run()
784+
if err == nil {
785+
return true, nil
786+
}
787+
var exitErr *exec.ExitError
788+
if errors.As(err, &exitErr) && exitErr.ExitCode() == 1 {
789+
return false, nil
790+
}
791+
return false, fmt.Errorf("git merge-base --is-ancestor %s %s: %w: %s", ancestor, descendant, err, strings.TrimSpace(stderr.String()))
792+
}
793+
794+
// commitTreeSHA returns the tree SHA recorded in the commit object at ref.
795+
func (m *gitMerger) commitTreeSHA(ctx context.Context, ref string) (string, error) {
796+
out, err := m.run(ctx, nil, "rev-parse", ref+"^{tree}")
797+
if err != nil {
798+
return "", fmt.Errorf("git rev-parse %s^{tree}: %w", ref, err)
799+
}
800+
return strings.TrimSpace(string(out)), nil
801+
}
802+
621803
// push pushes the current HEAD to refs/heads/<target> on the remote.
622804
func (m *gitMerger) push(ctx context.Context) error {
623805
refspec := "HEAD:refs/heads/" + m.target
@@ -750,12 +932,13 @@ func passthroughEnv(extra []string) []string {
750932
}
751933

752934
// isConcreteStrategy reports whether s names a concrete integration strategy
753-
// (i.e. not DEFAULT and not an unknown value). Only REBASE is implemented so
754-
// far; the remaining strategies are rejected as invalid requests until their
755-
// apply paths land.
935+
// (i.e. not DEFAULT and not an unknown value). PROMOTE is not implemented yet
936+
// and is rejected as an invalid request until its apply path lands.
756937
func isConcreteStrategy(s mergestrategypb.Strategy) bool {
757938
switch s {
758-
case mergestrategypb.Strategy_REBASE:
939+
case mergestrategypb.Strategy_REBASE,
940+
mergestrategypb.Strategy_SQUASH_REBASE,
941+
mergestrategypb.Strategy_MERGE:
759942
return true
760943
default:
761944
return false
@@ -770,6 +953,18 @@ func isRedundantCherryPick(out []byte) bool {
770953
strings.Contains(s, "nothing to commit")
771954
}
772955

956+
// squashMessage synthesizes a commit message for one squashed change. The wire
957+
// carries no upstream commit message, so the change is named by its provider
958+
// label — which is why this goes through the resolver rather than one
959+
// provider's fields, so a non-GitHub change is named too instead of being
960+
// silently omitted.
961+
func squashMessage(step *runwaymq.MergeStep, ref changeRef) string {
962+
if step.GetStepId() != "" {
963+
return fmt.Sprintf("squash: %s (%s)", step.GetStepId(), ref.Label)
964+
}
965+
return fmt.Sprintf("squash: %s", ref.Label)
966+
}
967+
773968
// toOutputs wraps commit SHAs as StepOutputs in order.
774969
func toOutputs(shas []string) []*runwaymq.StepOutput {
775970
if len(shas) == 0 {

0 commit comments

Comments
 (0)