Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions runway/extension/merger/git/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,10 @@ Fetching by SHA guarantees the merger applies exactly the commit a URI names —
| `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 |
| `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 |
| `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) |
| `PROMOTE` | Fast-forwards the target to an already-existing commit — no content transform, no new revision. Must be the entire request (one step, one change, one URI). | the exact named revision |
| `DEFAULT` | Resolved to the instance's configured default strategy before any step runs. | per the resolved strategy |

`PROMOTE` is defined by the wire contract but not yet applied here — a step naming it is rejected as an invalid request.
`PROMOTE` is exclusive because a pre-existing commit cannot descend from commits an earlier transforming step produced, and it advances the ref to an exact SHA rather than to the locally-built HEAD. Mixing it with any other step is rejected as an invalid request.

## Importing an unrelated history

Expand All @@ -58,11 +59,11 @@ Redelivery is safe: once imported, the source head is contained in the target, s

`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.

For a committing merge nothing reaches the remote until the final push. 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.
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.

## Failure classification

A merge conflict surfaces as `merger.ErrConflict`. An unusable request surfaces as `merger.ErrInvalidRequest`: an unsupported strategy or URI scheme, a malformed URI, a commit a reachable remote cannot supply, a change whose head has moved on, or a change sharing no history with the target under a picking strategy. Both are terminal — the controller publishes a `FAILED` result rather than retrying. Everything else (network/auth/push faults, and an unreachable remote) is returned as a plain error for the consumer to retry.
A merge conflict surfaces as `merger.ErrConflict`. An unusable request surfaces as `merger.ErrInvalidRequest`: an unsupported strategy or URI scheme, a malformed URI, an invalid `PROMOTE` composition, a commit a reachable remote cannot supply, a change whose head has moved on, or a change sharing no history with the target under a picking strategy. Both are terminal — the controller publishes a `FAILED` result rather than retrying. Everything else (network/auth/push faults, and an unreachable remote) is returned as a plain error for the consumer to retry.

The distinction between the last two matters operationally: a commit that is missing while the remote answers is a property of the request, whereas a remote that will not answer is a property of the moment.

Expand Down
129 changes: 118 additions & 11 deletions runway/extension/merger/git/git_merger.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,16 @@
// single commit (squash unit = the step).
// - MERGE: create a --no-ff merge commit per URI, preserving the
// original commit hashes in second-parent history.
// - PROMOTE: fast-forward the target to an already-existing commit with
// no content transform. PROMOTE must be the entire request (one step, one
// change, one URI) because a pre-existing commit cannot descend from
// commits produced by an earlier transforming step.
// - DEFAULT: resolved to the instance's configured DefaultStrategy
// before any step runs.
//
// PROMOTE is not implemented yet; a step naming it is rejected as
// merger.ErrInvalidRequest until its apply path lands.
//
// Atomicity: for a committing merge nothing reaches the remote until the final
// push. A step that fails to apply aborts the in-progress git operation and
// push (PROMOTE excepted, which is itself a single atomic fast-forward ref
// update). A step that fails to apply aborts the in-progress git operation and
// returns without pushing.
//
// Contention: if the push fails because the remote tip moved between reset and
Expand Down Expand Up @@ -116,7 +118,7 @@ type Params struct {
// Target is the destination branch ref on the remote (e.g. "main").
Target string
// DefaultStrategy resolves a step whose strategy is DEFAULT. Must be a
// concrete strategy (REBASE, SQUASH_REBASE, or MERGE).
// concrete strategy (REBASE, SQUASH_REBASE, MERGE, or PROMOTE).
DefaultStrategy mergestrategypb.Strategy
// Runtime is the pinned Git runtime used for every invocation.
Runtime GitRuntime
Expand Down Expand Up @@ -191,7 +193,7 @@ func NewMerger(params Params) (merger.Merger, error) {
return nil, err
}
if !isConcreteStrategy(params.DefaultStrategy) {
return nil, fmt.Errorf("default strategy must be concrete (REBASE, SQUASH_REBASE, or MERGE), got %v", params.DefaultStrategy)
return nil, fmt.Errorf("default strategy must be concrete (REBASE, SQUASH_REBASE, MERGE, or PROMOTE), got %v", params.DefaultStrategy)
}
maxAttempts := params.MaxPushAttempts
if maxAttempts <= 0 {
Expand Down Expand Up @@ -279,18 +281,24 @@ func (m *gitMerger) process(ctx context.Context, req *runwaymq.MergeRequest, com
"commit", commit,
)

// PROMOTE is exclusive: validated to be the entire request (one step).
if steps[0].strategy == mergestrategypb.Strategy_PROMOTE {
return m.promote(ctx, req, steps[0], commit)
}
return m.applyTransforming(ctx, req, steps, commit)
}

// resolveAndValidate normalizes DEFAULT strategies to the configured default
// and validates every change URI parses. All failures here are terminal (merger.ErrInvalidRequest): retrying never
// resolveAndValidate normalizes DEFAULT strategies to the configured default,
// validates every change URI parses, and enforces the PROMOTE composition rule.
// All failures here are terminal (merger.ErrInvalidRequest): retrying never
// succeeds, so the controller publishes a FAILED result rather than nacking.
func (m *gitMerger) resolveAndValidate(req *runwaymq.MergeRequest) ([]resolvedStep, error) {
if len(req.GetSteps()) == 0 {
return nil, fmt.Errorf("%w: request has no steps", merger.ErrInvalidRequest)
}

resolved := make([]resolvedStep, 0, len(req.GetSteps()))
promoteSeen := false
for _, step := range req.GetSteps() {
strategy := step.GetStrategy()
if strategy == mergestrategypb.Strategy_DEFAULT {
Expand All @@ -299,6 +307,10 @@ func (m *gitMerger) resolveAndValidate(req *runwaymq.MergeRequest) ([]resolvedSt
if !isConcreteStrategy(strategy) {
return nil, fmt.Errorf("%w: unsupported strategy %v", merger.ErrInvalidRequest, step.GetStrategy())
}
if strategy == mergestrategypb.Strategy_PROMOTE {
promoteSeen = true
}

ch := step.GetChange()
if ch == nil || len(ch.GetUris()) == 0 {
return nil, fmt.Errorf("%w: step %q has no change URIs", merger.ErrInvalidRequest, step.GetStepId())
Expand All @@ -311,6 +323,17 @@ func (m *gitMerger) resolveAndValidate(req *runwaymq.MergeRequest) ([]resolvedSt
resolved = append(resolved, resolvedStep{step: step, strategy: strategy})
}

// PROMOTE must be the entire request: one step, one change, one URI. A
// pre-existing commit cannot descend from commits an earlier transforming
// step produced, and PROMOTE pushes <sha>:target directly rather than
// HEAD:target, so it cannot compose with any other step.
if promoteSeen {
if len(resolved) != 1 ||
len(resolved[0].step.GetChange().GetUris()) != 1 {
return nil, fmt.Errorf("%w: PROMOTE must be the entire request (one step, one change, one URI)", merger.ErrInvalidRequest)
}
}

return resolved, nil
}

Expand Down Expand Up @@ -570,6 +593,75 @@ func (m *gitMerger) applyMerge(ctx context.Context, step *runwaymq.MergeStep) ([
return outputs, nil
}

// promote fast-forwards the target to an already-existing commit. It is only
// reachable for a validated single-step/single-change/single-URI request.
func (m *gitMerger) promote(ctx context.Context, req *runwaymq.MergeRequest, rs resolvedStep, commit bool) (*runwaymq.MergeResult, error) {
ref, err := resolveChange(rs.step.GetChange().GetUris()[0])
if err != nil {
return nil, err
}
sha := ref.SHA

var lastErr error
for attempt := 1; attempt <= m.maxPushAttempts; attempt++ {
if err := m.resetToRemote(ctx); err != nil {
return nil, err
}
// PROMOTE does not go through tryApply, so it performs the same
// availability and freshness checks itself. Without them a commit the
// remote cannot supply turns every containment query into a plain
// error, which the consumer retries forever instead of reporting.
if err := m.ensureObjects(ctx, []changeRef{ref}); err != nil {
return nil, err
}
if err := m.checkStale(ctx, []changeRef{ref}); err != nil {
return nil, err
}
tip, err := m.headSHA(ctx)
if err != nil {
return nil, err
}

// Idempotent: the commit is already the tip or contained in it.
if sha == tip {
return promoteResult(req, rs, sha, commit), nil
}
contained, err := m.isAncestor(ctx, sha, tip)
if err != nil {
return nil, err
}
if contained {
return promoteResult(req, rs, sha, commit), nil
}

// Only a true fast-forward is allowed; divergence is a terminal conflict.
fastForward, err := m.isAncestor(ctx, tip, sha)
if err != nil {
return nil, err
}
if !fastForward {
return nil, fmt.Errorf("%w: promote target %s is not a fast-forward of %s", merger.ErrConflict, sha, tip)
}

if !commit {
return promoteResult(req, rs, sha, commit), nil
}

refspec := sha + ":refs/heads/" + m.target
if _, err := m.run(ctx, nil, "push", m.remote, refspec); err != nil {
// The target may have moved under us; re-fetch and re-classify.
coremetrics.NamedCounter(m.metricsScope, "promote", "push_retries", 1)
m.logger.Warnw("promote push failed, re-classifying",
"attempt", attempt, "max_attempts", m.maxPushAttempts, "err", err)
lastErr = err
continue
}
return promoteResult(req, rs, sha, commit), nil
}
coremetrics.NamedCounter(m.metricsScope, "promote", "giveup", 1)
return nil, fmt.Errorf("exceeded %d promote attempts due to remote contention: %w", m.maxPushAttempts, lastErr)
}

// classifyMergeFailure decides what a failed `git merge` actually means, given
// whether the index was left holding conflicted entries.
//
Expand Down Expand Up @@ -956,13 +1048,13 @@ func passthroughEnv(extra []string) []string {
}

// isConcreteStrategy reports whether s names a concrete integration strategy
// (i.e. not DEFAULT and not an unknown value). PROMOTE is not implemented yet
// and is rejected as an invalid request until its apply path lands.
// (i.e. not DEFAULT and not an unknown value).
func isConcreteStrategy(s mergestrategypb.Strategy) bool {
switch s {
case mergestrategypb.Strategy_REBASE,
mergestrategypb.Strategy_SQUASH_REBASE,
mergestrategypb.Strategy_MERGE:
mergestrategypb.Strategy_MERGE,
mergestrategypb.Strategy_PROMOTE:
return true
default:
return false
Expand Down Expand Up @@ -1017,3 +1109,18 @@ func successResult(req *runwaymq.MergeRequest, steps []*runwaymq.StepResult) *ru
Steps: steps,
}
}

// promoteResult builds a SUCCEEDED MergeResult for a promote. A committing
// promote reports the promoted SHA as the step's single output; a dry-run check
// reports no output.
func promoteResult(req *runwaymq.MergeRequest, rs resolvedStep, sha string, commit bool) *runwaymq.MergeResult {
var outputs []*runwaymq.StepOutput
if commit {
outputs = []*runwaymq.StepOutput{{Id: sha}}
}
return &runwaymq.MergeResult{
Id: req.GetId(),
Outcome: runwaypb.Outcome_SUCCEEDED,
Steps: []*runwaymq.StepResult{{StepId: rs.step.GetStepId(), Outputs: outputs}},
}
}
119 changes: 117 additions & 2 deletions runway/extension/merger/git/git_merger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,85 @@ func TestMerge_Merge_AlreadyAncestor(t *testing.T) {

// --- PROMOTE ---

func TestMerge_Promote_MultiCommitChange(t *testing.T) {
// PROMOTE moves the ref to the named commit, so a change of any size
// arrives whole by construction — its ancestry comes along with it.
f := setupGitFixture(t)
head := f.pushMultiCommitPR(t, "feature/ff",
commitSpec{"a.txt", "a\n", "add a"},
commitSpec{"b.txt", "b\n", "add b"},
commitSpec{"c.txt", "c\n", "add c"},
)

m := f.newMerger(t, mergestrategypb.Strategy_REBASE)
res, err := m.Merge(context.Background(), req("b", stepOf(mergestrategypb.Strategy_PROMOTE, "s1", uri(head))))
require.NoError(t, err)
assert.Equal(t, head, f.remoteHEAD(t), "promote fast-forwards to the exact named commit")
assert.Equal(t, "a\n", f.remoteFile(t, "a.txt"))
assert.Equal(t, "c\n", f.remoteFile(t, "c.txt"))
require.Len(t, res.GetSteps(), 1)
require.Len(t, res.GetSteps()[0].GetOutputs(), 1)
assert.Equal(t, head, res.GetSteps()[0].GetOutputs()[0].GetId())
}

func TestMerge_Promote_UnavailableCommitIsInvalidNotRetryable(t *testing.T) {
// promote does not run through tryApply, so it needs its own availability
// check; otherwise the containment queries fail with a plain error and the
// consumer retries a request that can never succeed.
f := setupGitFixture(t)
m := f.newMerger(t, mergestrategypb.Strategy_REBASE)

_, err := m.Merge(context.Background(), req("b", stepOf(mergestrategypb.Strategy_PROMOTE, "s1", uri(fakeSHA))))
require.Error(t, err)
assert.True(t, errors.Is(err, merger.ErrInvalidRequest))
assert.False(t, errors.Is(err, merger.ErrConflict))
}

func TestMerge_Promote_FastForward(t *testing.T) {
f := setupGitFixture(t)
ffSHA := f.pushPRCommit(t, "feature/ff", "hello.txt", "hello\nearth\n", "ff")

m := f.newMerger(t, mergestrategypb.Strategy_REBASE)
res, err := m.Merge(context.Background(), req("b", stepOf(mergestrategypb.Strategy_PROMOTE, "s1", uri(ffSHA))))
require.NoError(t, err)
assert.Equal(t, runwaypb.Outcome_SUCCEEDED, res.GetOutcome())
require.Len(t, res.GetSteps(), 1)
require.Len(t, res.GetSteps()[0].GetOutputs(), 1)
assert.Equal(t, ffSHA, res.GetSteps()[0].GetOutputs()[0].GetId(), "promote reports the exact named SHA")
assert.Equal(t, ffSHA, f.remoteHEAD(t))
}

func TestMerge_Promote_AlreadyContained(t *testing.T) {
f := setupGitFixture(t)
seedSHA := f.remoteSHA(t, "main")
advSHA := f.pushPRCommit(t, "feature/adv", "adv.txt", "adv\n", "adv")
f.advanceMain(t, advSHA)
mainBefore := f.remoteHEAD(t)

m := f.newMerger(t, mergestrategypb.Strategy_REBASE)
res, err := m.Merge(context.Background(), req("b", stepOf(mergestrategypb.Strategy_PROMOTE, "s1", uri(seedSHA))))
require.NoError(t, err)
assert.Equal(t, runwaypb.Outcome_SUCCEEDED, res.GetOutcome())
assert.Equal(t, mainBefore, f.remoteHEAD(t), "promoting an already-contained SHA does not move the tip")
}

func TestMerge_Promote_Divergent(t *testing.T) {
f := setupGitFixture(t)
seedSHA := f.remoteSHA(t, "main")
divSHA := f.pushPRCommitFrom(t, seedSHA, "feature/div", "div.txt", "div\n", "div")
otherSHA := f.pushPRCommitFrom(t, seedSHA, "feature/other", "other.txt", "other\n", "other")
f.advanceMain(t, otherSHA)
mainBefore := f.remoteHEAD(t)

m := f.newMerger(t, mergestrategypb.Strategy_REBASE)
_, err := m.Merge(context.Background(), req("b", stepOf(mergestrategypb.Strategy_PROMOTE, "s1", uri(divSHA))))
require.Error(t, err)
assert.True(t, errors.Is(err, merger.ErrConflict))
assert.Equal(t, mainBefore, f.remoteHEAD(t))
}

// --- DEFAULT ---

func TestMerge_Default_ResolvesToRebase(t *testing.T) {
f := setupGitFixture(t)
sha := f.pushPRCommit(t, "feature/a", "hello.txt", "hello\nearth\n", "tweak hello")
Expand Down Expand Up @@ -966,8 +1045,15 @@ func TestMerge_InvalidRequests(t *testing.T) {
req: req("b"),
},
{
name: "unsupported strategy",
req: req("b", stepOf(mergestrategypb.Strategy_PROMOTE, "s1", uri(fakeSHA))),
name: "promote with two steps",
req: req("b",
stepOf(mergestrategypb.Strategy_PROMOTE, "s1", uri(fakeSHA)),
stepOf(mergestrategypb.Strategy_PROMOTE, "s2", uri(fakeSHA)),
),
},
{
name: "promote step with two URIs",
req: req("b", stepOf(mergestrategypb.Strategy_PROMOTE, "s1", uri(fakeSHA), uri(fakeSHA))),
},
{
name: "malformed URI",
Expand Down Expand Up @@ -1027,6 +1113,35 @@ func TestCheckMergeability_Conflict(t *testing.T) {
assert.Equal(t, mainBefore, f.remoteHEAD(t))
}

func TestCheckMergeability_PromoteFastForward(t *testing.T) {
f := setupGitFixture(t)
ffSHA := f.pushPRCommit(t, "feature/ff", "hello.txt", "hello\nearth\n", "ff")
mainBefore := f.remoteHEAD(t)

m := f.newMerger(t, mergestrategypb.Strategy_REBASE)
res, err := m.CheckMergeability(context.Background(), req("b", stepOf(mergestrategypb.Strategy_PROMOTE, "s1", uri(ffSHA))))
require.NoError(t, err)
assert.Equal(t, runwaypb.Outcome_SUCCEEDED, res.GetOutcome())
require.Len(t, res.GetSteps(), 1)
assert.Empty(t, res.GetSteps()[0].GetOutputs())
assert.Equal(t, mainBefore, f.remoteHEAD(t))
}

func TestCheckMergeability_PromoteDivergent(t *testing.T) {
f := setupGitFixture(t)
seedSHA := f.remoteSHA(t, "main")
divSHA := f.pushPRCommitFrom(t, seedSHA, "feature/div", "div.txt", "div\n", "div")
otherSHA := f.pushPRCommitFrom(t, seedSHA, "feature/other", "other.txt", "other\n", "other")
f.advanceMain(t, otherSHA)
mainBefore := f.remoteHEAD(t)

m := f.newMerger(t, mergestrategypb.Strategy_REBASE)
_, err := m.CheckMergeability(context.Background(), req("b", stepOf(mergestrategypb.Strategy_PROMOTE, "s1", uri(divSHA))))
require.Error(t, err)
assert.True(t, errors.Is(err, merger.ErrConflict))
assert.Equal(t, mainBefore, f.remoteHEAD(t))
}

func TestPinnedGitVersion(t *testing.T) {
out := mustGitOutput(t, t.TempDir(), "--version")
assert.Equal(t, "git version "+pinnedGitVersion, strings.TrimSpace(string(out)))
Expand Down
Loading