Skip to content

Commit b993dff

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 transforming strategies on top of the shared apply machinery. ### What? **SQUASH_REBASE** applies the step exactly like REBASE — cherry-picking each URI's head commit onto the tip — then collapses the commits the step produced into a single commit. The squash unit is the step, not the request: a multi-step request still lands one commit per step. Two degenerate cases produce no output rather than an empty commit. If every change in the step was already present the picks moved HEAD nowhere, so there is nothing to squash. If the picks did create commits but their net tree matches the base, the squashed result would be empty, so the intermediates are dropped and the step reports no output. Both keep redelivery idempotent. **MERGE** creates a `--no-ff` merge commit per URI, which keeps the original commit hashes reachable through second-parent history — the difference that matters versus REBASE, where cherry-picking rewrites them. A SHA already contained in HEAD is skipped rather than merged again. A merge that conflicts aborts the in-progress merge before returning `ErrConflict`, leaving the checkout usable for the next request. Both strategies join the existing dispatch and inherit the reset/apply/push cycle, contention retry, and dry-run discard unchanged. `isConcreteStrategy` now admits them; PROMOTE remains rejected until the next change in the stack. Restores `isAncestor` (containment checks for MERGE) and adds `squashMessage`, which synthesizes the squash commit message from the step id and the PRs named by its URIs — only SHAs are on the wire, so there is no upstream message to carry over. ## Test Plan ✅ `bazel test //runway/extension/merger/...` — 2/2 pass (git suite 24.7s) New cases: SQUASH_REBASE collapsing two stacked URIs into one output, SQUASH_REBASE over an already-landed change producing none, MERGE creating a merge commit for a fresh change, MERGE skipping a SHA already an ancestor of the tip, and the MERGE dry-run path.
1 parent ffa903d commit b993dff

3 files changed

Lines changed: 230 additions & 14 deletions

File tree

runway/extension/merger/git/README.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,11 @@ The URI carries only the head commit SHA — no base and no commit range. A chan
1313
| Strategy | What it does | Outputs |
1414
|---|---|---|
1515
| `REBASE` | Cherry-picks each URI's head commit onto the tip, in order. A pick already present on the target is rebased out (reported, no output); an empty pick is rolled back. | one revision per newly-created commit |
16+
| `SQUASH_REBASE` | Applies the step like `REBASE`, then collapses the resulting commits into a single commit (squash unit = the step). | one revision, or none when the step is entirely already-present |
17+
| `MERGE` | Creates a `--no-ff` merge commit per URI, preserving the original commit hashes in second-parent history. A commit already contained in the tip is skipped. | the merge-commit revision(s) |
1618
| `DEFAULT` | Resolved to the instance's configured default strategy before any step runs. | per the resolved strategy |
1719

18-
`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.
20+
`PROMOTE` is defined by the wire contract but not yet applied here — a step naming it is rejected as an invalid request.
1921

2022
## Committing, dry-run, atomicity, contention
2123

@@ -29,6 +31,6 @@ A merge conflict surfaces as `merger.ErrConflict`; an unusable request (unsuppor
2931

3032
## Runtime and identity
3133

32-
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.
34+
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.
3335

3436
Object availability relies on `git fetch <remote>` making the referenced SHAs reachable through the remote's refs. A deployment whose head commits live only under non-fetched refs (for example GitHub `refs/pull/*`) must arrange for those objects to be fetchable.

runway/extension/merger/git/git_merger.go

Lines changed: 140 additions & 11 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
@@ -73,8 +77,8 @@ const defaultMaxPushAttempts = 10
7377

7478
// Default committer identity used when Params leaves it unset. The scrubbed
7579
// environment (GIT_CONFIG_NOSYSTEM, GIT_CONFIG_GLOBAL=/dev/null) leaves no
76-
// ambient identity, so the commit-creating REBASE strategy needs one supplied
77-
// explicitly.
80+
// ambient identity, so commit-creating strategies (REBASE/SQUASH_REBASE/MERGE)
81+
// need one supplied explicitly.
7882
const (
7983
defaultCommitterName = "SubmitQueue Runway"
8084
defaultCommitterEmail = "runway@submitqueue.invalid"
@@ -102,7 +106,7 @@ type Params struct {
102106
// Target is the destination branch ref on the remote (e.g. "main").
103107
Target string
104108
// DefaultStrategy resolves a step whose strategy is DEFAULT. Must be a
105-
// concrete strategy (currently only REBASE).
109+
// concrete strategy (REBASE, SQUASH_REBASE, or MERGE).
106110
DefaultStrategy mergestrategypb.Strategy
107111
// Runtime is the pinned Git runtime used for every invocation.
108112
Runtime GitRuntime
@@ -156,7 +160,7 @@ func NewMerger(params Params) (merger.Merger, error) {
156160
return nil, err
157161
}
158162
if !isConcreteStrategy(params.DefaultStrategy) {
159-
return nil, fmt.Errorf("default strategy must be concrete (currently only REBASE), got %v", params.DefaultStrategy)
163+
return nil, fmt.Errorf("default strategy must be concrete (REBASE, SQUASH_REBASE, or MERGE), got %v", params.DefaultStrategy)
160164
}
161165
maxAttempts := params.MaxPushAttempts
162166
if maxAttempts <= 0 {
@@ -276,7 +280,8 @@ func (m *gitMerger) resolveAndValidate(req *runwaymq.MergeRequest) ([]resolvedSt
276280
}
277281

278282
// applyTransforming runs the reset/apply/push cycle for the transforming
279-
// strategies, retrying on remote contention when committing. For a dry run it applies the steps locally then discards them.
283+
// strategies (REBASE, SQUASH_REBASE, MERGE), retrying on remote contention when
284+
// committing. For a dry run it applies the steps locally then discards them.
280285
func (m *gitMerger) applyTransforming(ctx context.Context, req *runwaymq.MergeRequest, steps []resolvedStep, commit bool) (*runwaymq.MergeResult, error) {
281286
var lastErr error
282287
for attempt := 1; attempt <= m.maxPushAttempts; attempt++ {
@@ -372,6 +377,10 @@ func (m *gitMerger) applySteps(ctx context.Context, steps []resolvedStep) ([]*ru
372377
switch rs.strategy {
373378
case mergestrategypb.Strategy_REBASE:
374379
outputs, err = m.applyRebase(ctx, rs.step)
380+
case mergestrategypb.Strategy_SQUASH_REBASE:
381+
outputs, err = m.applySquashRebase(ctx, rs.step)
382+
case mergestrategypb.Strategy_MERGE:
383+
outputs, err = m.applyMerge(ctx, rs.step)
375384
default:
376385
// resolveAndValidate rejects anything else; defensive.
377386
return nil, fmt.Errorf("%w: unsupported strategy %v", merger.ErrInvalidRequest, rs.strategy)
@@ -394,6 +403,92 @@ func (m *gitMerger) applyRebase(ctx context.Context, step *runwaymq.MergeStep) (
394403
return toOutputs(picked), nil
395404
}
396405

406+
// applySquashRebase cherry-picks the step's changes like REBASE, then collapses
407+
// the resulting commits into a single squashed commit (squash unit = the step).
408+
// A step whose changes are all already present, or whose net change is empty,
409+
// produces no output.
410+
func (m *gitMerger) applySquashRebase(ctx context.Context, step *runwaymq.MergeStep) ([]*runwaymq.StepOutput, error) {
411+
preSHA, err := m.headSHA(ctx)
412+
if err != nil {
413+
return nil, err
414+
}
415+
if _, err := m.pickStepChanges(ctx, step); err != nil {
416+
return nil, err
417+
}
418+
postSHA, err := m.headSHA(ctx)
419+
if err != nil {
420+
return nil, err
421+
}
422+
if postSHA == preSHA {
423+
// Every change was already present; nothing to squash.
424+
return nil, nil
425+
}
426+
427+
// If the picks produced commits but their net tree matches the base, the
428+
// squashed result would be empty — drop the intermediates, no output.
429+
preTree, err := m.commitTreeSHA(ctx, preSHA)
430+
if err != nil {
431+
return nil, err
432+
}
433+
postTree, err := m.commitTreeSHA(ctx, postSHA)
434+
if err != nil {
435+
return nil, err
436+
}
437+
if preTree == postTree {
438+
if _, err := m.run(ctx, nil, "reset", "--hard", preSHA); err != nil {
439+
return nil, fmt.Errorf("git reset --hard %s after empty squash: %w", preSHA, err)
440+
}
441+
return nil, nil
442+
}
443+
444+
if _, err := m.run(ctx, nil, "reset", "--soft", preSHA); err != nil {
445+
return nil, fmt.Errorf("git reset --soft %s: %w", preSHA, err)
446+
}
447+
if _, err := m.run(ctx, nil, "commit", "-m", squashMessage(step)); err != nil {
448+
return nil, fmt.Errorf("git commit (squash): %w", err)
449+
}
450+
sha, err := m.headSHA(ctx)
451+
if err != nil {
452+
return nil, err
453+
}
454+
return []*runwaymq.StepOutput{{Id: sha}}, nil
455+
}
456+
457+
// applyMerge creates a --no-ff merge commit for the head SHA of every URI of
458+
// every change in the step, preserving the original commit hashes in
459+
// second-parent history. A SHA already contained in HEAD produces no output.
460+
func (m *gitMerger) applyMerge(ctx context.Context, step *runwaymq.MergeStep) ([]*runwaymq.StepOutput, error) {
461+
var outputs []*runwaymq.StepOutput
462+
for _, uri := range step.GetChange().GetUris() {
463+
cid, err := entitygithub.ParseChangeID(uri)
464+
if err != nil {
465+
return nil, fmt.Errorf("%w: invalid change URI %q: %v", merger.ErrInvalidRequest, uri, err)
466+
}
467+
sha := cid.HeadCommitSHA
468+
469+
contained, err := m.isAncestor(ctx, sha, "HEAD")
470+
if err != nil {
471+
return nil, err
472+
}
473+
if contained {
474+
continue
475+
}
476+
477+
out, err := m.runCombined(ctx, nil, "merge", "--no-ff", "--no-edit", sha)
478+
if err != nil {
479+
_, _ = m.run(ctx, nil, "merge", "--abort")
480+
coremetrics.NamedCounter(m.metricsScope, "merge", "merge_conflicts", 1)
481+
return nil, fmt.Errorf("%w: git merge %s: %s", merger.ErrConflict, sha, strings.TrimSpace(string(out)))
482+
}
483+
mergeSHA, err := m.headSHA(ctx)
484+
if err != nil {
485+
return nil, err
486+
}
487+
outputs = append(outputs, &runwaymq.StepOutput{Id: mergeSHA})
488+
}
489+
return outputs, nil
490+
}
491+
397492
// pickStepChanges cherry-picks the head SHA of every URI of every change in the
398493
// step, in order, returning the new commit SHAs (empty for picks that were
399494
// no-ops because the content is already on the target).
@@ -494,6 +589,24 @@ func (m *gitMerger) refetchTipSHA(ctx context.Context) (string, error) {
494589
return strings.TrimSpace(string(out)), nil
495590
}
496591

592+
// isAncestor reports whether ancestor is an ancestor of (or equal to)
593+
// descendant. `git merge-base --is-ancestor` exits 0 for true, 1 for false;
594+
// any other exit is a real error.
595+
func (m *gitMerger) isAncestor(ctx context.Context, ancestor, descendant string) (bool, error) {
596+
cmd := m.command(ctx, "merge-base", "--is-ancestor", ancestor, descendant)
597+
var stderr bytes.Buffer
598+
cmd.Stderr = &stderr
599+
err := cmd.Run()
600+
if err == nil {
601+
return true, nil
602+
}
603+
var exitErr *exec.ExitError
604+
if errors.As(err, &exitErr) && exitErr.ExitCode() == 1 {
605+
return false, nil
606+
}
607+
return false, fmt.Errorf("git merge-base --is-ancestor %s %s: %w: %s", ancestor, descendant, err, strings.TrimSpace(stderr.String()))
608+
}
609+
497610
// isEmptyHEADCommit returns true when HEAD's tree matches HEAD^'s tree — i.e.
498611
// the most recent commit introduces no changes.
499612
func (m *gitMerger) isEmptyHEADCommit(ctx context.Context) (bool, error) {
@@ -601,12 +714,13 @@ func newGitCommand(ctx context.Context, runtime GitRuntime, dir string, args ...
601714
}
602715

603716
// isConcreteStrategy reports whether s names a concrete integration strategy
604-
// (i.e. not DEFAULT and not an unknown value). Only REBASE is implemented so
605-
// far; the remaining strategies are rejected as invalid requests until their
606-
// apply paths land.
717+
// (i.e. not DEFAULT and not an unknown value). PROMOTE is not implemented yet
718+
// and is rejected as an invalid request until its apply path lands.
607719
func isConcreteStrategy(s mergestrategypb.Strategy) bool {
608720
switch s {
609-
case mergestrategypb.Strategy_REBASE:
721+
case mergestrategypb.Strategy_REBASE,
722+
mergestrategypb.Strategy_SQUASH_REBASE,
723+
mergestrategypb.Strategy_MERGE:
610724
return true
611725
default:
612726
return false
@@ -621,6 +735,21 @@ func isRedundantCherryPick(out []byte) bool {
621735
strings.Contains(s, "nothing to commit")
622736
}
623737

738+
// squashMessage synthesizes a commit message for a squashed step, referencing
739+
// the step id and the PRs of its changes (only SHAs are available on the wire).
740+
func squashMessage(step *runwaymq.MergeStep) string {
741+
var prs []string
742+
for _, uri := range step.GetChange().GetUris() {
743+
if cid, err := entitygithub.ParseChangeID(uri); err == nil {
744+
prs = append(prs, fmt.Sprintf("%s#%d", cid.OwnerRepo(), cid.PRNumber))
745+
}
746+
}
747+
if step.GetStepId() != "" {
748+
return fmt.Sprintf("squash: %s (%s)", step.GetStepId(), strings.Join(prs, ", "))
749+
}
750+
return fmt.Sprintf("squash: %s", strings.Join(prs, ", "))
751+
}
752+
624753
// toOutputs wraps commit SHAs as StepOutputs in order.
625754
func toOutputs(shas []string) []*runwaymq.StepOutput {
626755
if len(shas) == 0 {

runway/extension/merger/git/git_merger_test.go

Lines changed: 86 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,77 @@ func TestMerge_Rebase_GivesUpAfterMaxAttempts(t *testing.T) {
352352
assert.Equal(t, raceSHAs[1], f.remoteHEAD(t))
353353
}
354354

355+
// --- SQUASH_REBASE ---
356+
357+
func TestMerge_SquashRebase_TwoStackedURIsOneOutput(t *testing.T) {
358+
f := setupGitFixture(t)
359+
sha1, sha2 := f.pushStack(t)
360+
361+
m := f.newMerger(t, mergestrategypb.Strategy_REBASE)
362+
res, err := m.Merge(context.Background(), req("b",
363+
stepOf(mergestrategypb.Strategy_SQUASH_REBASE, "s1", uri(sha1), uri(sha2)),
364+
))
365+
require.NoError(t, err)
366+
assert.Equal(t, runwaypb.Outcome_SUCCEEDED, res.GetOutcome())
367+
require.Len(t, res.GetSteps(), 1)
368+
require.Len(t, res.GetSteps()[0].GetOutputs(), 1, "the two picks collapse to a single squashed commit")
369+
370+
commits := f.remoteCommitsSinceSeed(t)
371+
require.Len(t, commits, 1, "exactly one new commit on the remote")
372+
assert.Equal(t, res.GetSteps()[0].GetOutputs()[0].GetId(), commits[0])
373+
assert.Equal(t, "hello\nearth\ngoodbye\n", f.remoteFile(t, "hello.txt"))
374+
}
375+
376+
func TestMerge_SquashRebase_AlreadyLanded(t *testing.T) {
377+
f := setupGitFixture(t)
378+
sha := f.pushPRCommit(t, "feature/a", "hello.txt", "hello\nearth\n", "tweak hello")
379+
f.landOnMain(t, sha)
380+
mainBefore := f.remoteHEAD(t)
381+
382+
m := f.newMerger(t, mergestrategypb.Strategy_REBASE)
383+
res, err := m.Merge(context.Background(), req("b", stepOf(mergestrategypb.Strategy_SQUASH_REBASE, "s1", uri(sha))))
384+
require.NoError(t, err)
385+
require.Len(t, res.GetSteps(), 1)
386+
assert.Empty(t, res.GetSteps()[0].GetOutputs())
387+
assert.Equal(t, mainBefore, f.remoteHEAD(t))
388+
}
389+
390+
// --- MERGE ---
391+
392+
func TestMerge_Merge_FreshChangeCreatesMergeCommit(t *testing.T) {
393+
f := setupGitFixture(t)
394+
freshSHA := f.pushPRCommit(t, "feature/a", "hello.txt", "hello\nearth\n", "tweak hello")
395+
396+
m := f.newMerger(t, mergestrategypb.Strategy_REBASE)
397+
res, err := m.Merge(context.Background(), req("b", stepOf(mergestrategypb.Strategy_MERGE, "s1", uri(freshSHA))))
398+
require.NoError(t, err)
399+
assert.Equal(t, runwaypb.Outcome_SUCCEEDED, res.GetOutcome())
400+
require.Len(t, res.GetSteps(), 1)
401+
require.Len(t, res.GetSteps()[0].GetOutputs(), 1)
402+
403+
mergeSHA := res.GetSteps()[0].GetOutputs()[0].GetId()
404+
assert.Equal(t, mergeSHA, f.remoteHEAD(t))
405+
assert.Len(t, f.parents(t, mergeSHA), 2, "a --no-ff merge commit has two parents")
406+
assert.Contains(t, f.parents(t, mergeSHA), freshSHA,
407+
"the original head commit is preserved as the merge's second parent")
408+
}
409+
410+
func TestMerge_Merge_AlreadyAncestor(t *testing.T) {
411+
f := setupGitFixture(t)
412+
freshSHA := f.pushPRCommit(t, "feature/a", "hello.txt", "hello\nearth\n", "tweak hello")
413+
f.advanceMain(t, freshSHA) // fast-forward main directly to freshSHA
414+
mainBefore := f.remoteHEAD(t)
415+
416+
m := f.newMerger(t, mergestrategypb.Strategy_REBASE)
417+
res, err := m.Merge(context.Background(), req("b", stepOf(mergestrategypb.Strategy_MERGE, "s1", uri(freshSHA))))
418+
require.NoError(t, err)
419+
require.Len(t, res.GetSteps(), 1)
420+
assert.Empty(t, res.GetSteps()[0].GetOutputs())
421+
assert.Equal(t, mainBefore, f.remoteHEAD(t))
422+
}
423+
424+
// --- PROMOTE ---
425+
355426
func TestMerge_Default_ResolvesToRebase(t *testing.T) {
356427
f := setupGitFixture(t)
357428
sha := f.pushPRCommit(t, "feature/a", "hello.txt", "hello\nearth\n", "tweak hello")
@@ -378,7 +449,7 @@ func TestMerge_InvalidRequests(t *testing.T) {
378449
},
379450
{
380451
name: "unsupported strategy",
381-
req: req("b", stepOf(mergestrategypb.Strategy_MERGE, "s1", uri(fakeSHA))),
452+
req: req("b", stepOf(mergestrategypb.Strategy_PROMOTE, "s1", uri(fakeSHA))),
382453
},
383454
{
384455
name: "malformed URI",
@@ -413,6 +484,20 @@ func TestCheckMergeability_RebaseMergeable(t *testing.T) {
413484
assert.Equal(t, mainBefore, f.remoteHEAD(t), "a dry run does not advance the remote tip")
414485
}
415486

487+
func TestCheckMergeability_MergeMergeable(t *testing.T) {
488+
f := setupGitFixture(t)
489+
sha := f.pushPRCommit(t, "feature/a", "hello.txt", "hello\nearth\n", "tweak hello")
490+
mainBefore := f.remoteHEAD(t)
491+
492+
m := f.newMerger(t, mergestrategypb.Strategy_REBASE)
493+
res, err := m.CheckMergeability(context.Background(), req("b", stepOf(mergestrategypb.Strategy_MERGE, "s1", uri(sha))))
494+
require.NoError(t, err)
495+
assert.Equal(t, runwaypb.Outcome_SUCCEEDED, res.GetOutcome())
496+
require.Len(t, res.GetSteps(), 1)
497+
assert.Empty(t, res.GetSteps()[0].GetOutputs())
498+
assert.Equal(t, mainBefore, f.remoteHEAD(t))
499+
}
500+
416501
func TestCheckMergeability_Conflict(t *testing.T) {
417502
f := setupGitFixture(t)
418503
mainBefore, conflictingSHA := f.setupConflict(t)

0 commit comments

Comments
 (0)