Skip to content
Merged
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
2 changes: 1 addition & 1 deletion doc/rfc/runway/workflow.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ Together these guarantee the client's correlation id always resolves: the primar

Runway has no persistent state — no request store, no job store, no database. Idempotency is achieved through the VCS contract: merge detects already-pushed changes (revisions reachable from HEAD) and treats them as already-landed. Merge-conflict check is read-only and naturally idempotent.

A committing merge is also atomic against the merge target: it updates the target at most once per request, and afterwards either every step of the request is reachable from the target or the target is unchanged. A retried redelivery therefore either replays cleanly against the same unchanged target or finds its work already landed.
Each step of a committing merge lands atomically and in order, and the merge stops at the first step that fails: ordering cannot be guaranteed past a failure, so later steps are not attempted, and the steps before it stay landed and are reported in the `FAILED` result. A retried redelivery therefore finds the steps that landed already on the target and skips them.

## Ownership by service

Expand Down
8 changes: 7 additions & 1 deletion runway/controller/merge/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,11 +125,17 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er
"queue_name", request.GetQueueName(),
)
}
result = &runwaymq.MergeResult{
failed := &runwaymq.MergeResult{
Id: request.GetId(),
Outcome: runwaypb.Outcome_FAILED,
Reason: err.Error(),
}
// A merger that lands step by step reports the steps that landed before
// the failure; they stay landed, so the caller must see them.
if result != nil {
failed.Steps = result.GetSteps()
}
result = failed
}

// Echo the request's queue name so the consumer can route the result by
Expand Down
55 changes: 55 additions & 0 deletions runway/controller/merge/merge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,61 @@ func TestProcess_MergeConflict(t *testing.T) {
assert.NotEmpty(t, result.Reason)
}

func TestProcess_PartialLandIsReportedOnFailure(t *testing.T) {
ctrl := gomock.NewController(t)

partial := &runwaymq.MergeResult{
Id: testID,
Outcome: runwaypb.Outcome_FAILED,
Steps: []*runwaymq.StepResult{
{StepId: "step-1", Outputs: []*runwaymq.StepOutput{{Id: "abc123"}}},
{StepId: "step-2", Reason: "conflict"},
},
}
m := mergermock.NewMockMerger(ctrl)
m.EXPECT().Merge(gomock.Any(), gomock.Any()).Return(partial, fmt.Errorf("step-2: %w", merger.ErrConflict))

factory := mergermock.NewMockFactory(ctrl)
factory.EXPECT().For(merger.Config{QueueName: testQueue}).Return(m, nil)

var gotPayload []byte
pub := queuemock.NewMockPublisher(ctrl)
pub.EXPECT().Publish(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(
func(_ context.Context, _ string, msg entityqueue.Message) error {
gotPayload = msg.Payload
return nil
},
)
q := queuemock.NewMockQueue(ctrl)
q.EXPECT().Publisher().Return(pub).AnyTimes()
registry, err := consumer.NewTopicRegistry([]consumer.TopicConfig{
{Key: runwaymq.TopicKeyMergeSignal, Name: "merge-signal", Queue: q},
})
require.NoError(t, err)

controller := newController(t, factory, registry)

req := &runwaymq.MergeRequest{
Id: testID,
QueueName: testQueue,
Steps: []*runwaymq.MergeStep{{StepId: "step-1"}, {StepId: "step-2"}, {StepId: "step-3"}},
}
delivery := newDelivery(t, ctrl, requestPayload(t, req))

require.NoError(t, controller.Process(context.Background(), delivery))

result := &runwaymq.MergeResult{}
require.NoError(t, runwaymq.Unmarshal(gotPayload, result))
assert.Equal(t, runwaypb.Outcome_FAILED, result.Outcome)
assert.NotEmpty(t, result.Reason)
require.Len(t, result.Steps, 2)
assert.Equal(t, "step-1", result.Steps[0].StepId)
require.Len(t, result.Steps[0].Outputs, 1)
assert.Equal(t, "abc123", result.Steps[0].Outputs[0].Id)
assert.Equal(t, "step-2", result.Steps[1].StepId)
assert.NotEmpty(t, result.Steps[1].Reason)
}

func TestProcess_InvalidRequest(t *testing.T) {
ctrl := gomock.NewController(t)

Expand Down
44 changes: 44 additions & 0 deletions runway/extension/merger/github/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
load("@rules_go//go:def.bzl", "go_library", "go_test")

go_library(
name = "go_default_library",
srcs = [
"changeref.go",
"classifier.go",
"client.go",
"github_merger.go",
],
importpath = "github.com/uber/submitqueue/runway/extension/merger/github",
visibility = ["//visibility:public"],
deps = [
"//api/base/mergestrategy/protopb:go_default_library",
"//api/runway/messagequeue:go_default_library",
"//api/runway/messagequeue/protopb:go_default_library",
"//platform/base/change/github:go_default_library",
"//platform/errs:go_default_library",
"//platform/http:go_default_library",
"//platform/metrics:go_default_library",
"//runway/extension/merger:go_default_library",
"@com_github_uber_go_tally//:go_default_library",
"@org_uber_go_zap//:go_default_library",
],
)

go_test(
name = "go_default_test",
srcs = ["github_merger_test.go"],
embed = [":go_default_library"],
deps = [
"//api/base/change/protopb:go_default_library",
"//api/base/mergestrategy/protopb:go_default_library",
"//api/runway/messagequeue:go_default_library",
"//api/runway/messagequeue/protopb:go_default_library",
"//platform/errs:go_default_library",
"//platform/http:go_default_library",
"//runway/extension/merger:go_default_library",
"@com_github_stretchr_testify//assert:go_default_library",
"@com_github_stretchr_testify//require:go_default_library",
"@com_github_uber_go_tally//:go_default_library",
"@org_uber_go_zap//zaptest:go_default_library",
],
)
51 changes: 51 additions & 0 deletions runway/extension/merger/github/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# github merger

A `merger.Merger` that lands `github://` changes through the GitHub REST API instead of a local checkout. GitHub performs the merge itself, so branch rules, the pull request's merged state, merge commit attribution and the rebasing of a stack's remaining pull requests are all GitHub's. It is constructed by the wiring layer (see [`service/runway`](../../../../service/runway)) with an HTTP client, the repository it serves (host, owner, repo), the trunk branch and the default strategy.

## Model

A request is an ordered list of steps, and a step's change is an ordered list of URIs — a stack, bottom first. This merger maps that directly onto GitHub's [stacked pull requests](https://docs.github.com/en/pull-requests/get-started/about-stacked-prs): **one step is one GitHub stack**, and a single pull request is the one-element case.

A step is landed with one call to the [asynchronous merge API](https://docs.github.com/en/rest/pulls/pulls#merge-a-pull-request-asynchronously) on the step's top pull request. For a stacked pull request GitHub merges it together with every unmerged pull request below it, as one operation that either lands all of them or none, ordered bottom-up in the resulting history. Pull requests above the step's top stay open and are rebased onto the trunk by GitHub. The merge runs with `direct_merge`: Runway is the queue, so handing the pull requests to GitHub's own merge queue would let a second queue reorder what this one decided.

Each step's strategy maps onto a GitHub merge method — `REBASE` to `rebase`, `SQUASH_REBASE` to `squash`, `MERGE` to `merge` — and `DEFAULT` resolves to the configured default first. `PROMOTE` advances a ref to an existing revision, which no pull request merge expresses, so it is an invalid request here; the git merger serves it.

Each URI's output is the merge commit GitHub records for its pull request: the squash commit, the merge commit, or, for a rebase, the last commit the rebase created for that pull request. That is one output per URI, where the git merger reports one per created commit under `REBASE`. It is read from the pull request's `merged` issue event, because API version 2026-03-10 no longer reports `merge_commit_sha` on a merged pull request. GitHub reports a stack merge settled a moment before every pull request in it shows its merge, so the merger re-reads until each is recorded.

## What a step must be

The URIs of a step must be something GitHub will land as one stack onto the target, and anything else is refused as an invalid request:

- Every URI names a pull request in the configured repository on the configured host, at the head commit the pull request still has. A head that moved is stale; a closed, unmerged pull request or a draft cannot be landed.
- Pull requests that are already merged must be a prefix of the list, since a stack merges bottom-up; they are skipped.
- One remaining pull request must be based on the target. A stacked pull request listed on its own is based on the one below it, and merging it would land that one too.
- Several remaining pull requests must be the unmerged bottom of one GitHub stack based on the target, in the stack's order and with no gap. Separate pull requests that each target the trunk, a stack listed out of order or with a pull request missing, and pull requests from two stacks are all refused. The merger never creates or edits a stack on the author's behalf.

These checks run at the mergeability check, so SubmitQueue rejects such a request at validation rather than after batching it. `Merge` runs them again, before submitting anything, because a stack can be edited between validation and landing.

## Mergeability check

`CheckMergeability` writes nothing. After the checks above it reads GitHub's `mergeable` verdict for every remaining pull request, re-reading while GitHub reports it as still being computed; a pull request that is not mergeable, or whose `mergeable_state` is `dirty`, is a conflict.

GitHub computes that verdict for each pull request against its own base, so a check sees conflicts within a stack and against the trunk, but not between the steps of one request. Those surface when the merge is attempted.

## Atomicity

Each step is one GitHub merge, so each step lands atomically. Steps land in request order, and `Merge` stops at the first that fails: ordering cannot be guaranteed past a failure, so later steps are not attempted, and the steps before it stay landed. The `FAILED` result lists a `StepResult` per landed step with its outputs, then one for the failed step with its reason.

## Idempotency and redelivery

A redelivered request converges instead of merging twice. Already-merged pull requests are skipped and report their recorded merge commit, so a step that fully landed before a crash is reported without another call. An already-merged pull request is trusted as landed on the target; no further check is made, since one would cost a call per pull request on every redelivery. A submission GitHub already has in flight answers with that request's id (HTTP 409); it is adopted and polled only when it merges the same head with the same method, and is an invalid request otherwise.

## Failure classification

- `merger.ErrInvalidRequest` (terminal): a malformed or foreign URI, an unsupported strategy, a stale head, a closed or draft pull request, a step that is not a stack based on the target, or GitHub refusing the merge as asked (HTTP 400/422, e.g. required checks not satisfied).
- `merger.ErrConflict` (terminal): a pull request that is not mergeable at the check, or a merge GitHub reports as failed. GitHub does not separate a conflict from other merge failures there; its message is the reason.
- `ErrMergePending` (retryable, via this package's `Classifier`): GitHub had not settled a merge, or a pull request's mergeability, within the poll budget. The work is still in flight on GitHub's side.
- Anything else is a plain error. HTTP rejections keep their status code (`platform/http.StatusError`) so `platform/errs/http` can retry a 5xx or 429 and dead-letter a 403 or 404. A 404 is not treated as terminal here: GitHub answers 404 for a repository the token cannot see, and that is a deployment fault, not a property of the request.

## Auth and hosts

The merger adds no credential and no base URL. Its HTTP client's transport roots relative paths at the API (for example `platform/http.NewClient("https://api.github.com")`) and authenticates them, so the wiring decides the scheme — a static token, an App installation token source, or anything an internal deployment injects. The token needs write access to the repository, and the right to bypass branch rules only when `BypassRules` is set.

The stacks and asynchronous merge APIs exist on github.com. GitHub Enterprise Server has not shipped stacked pull requests yet; the host, repository and API base URL are configuration, so an Enterprise Server instance needs no code change once it does.
49 changes: 49 additions & 0 deletions runway/extension/merger/github/changeref.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Copyright (c) 2026 Uber Technologies, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package github

import (
"fmt"

entitygithub "github.com/uber/submitqueue/platform/base/change/github"
"github.com/uber/submitqueue/runway/extension/merger"
)

// changeRef is a change URI reduced to the pull request it names.
type changeRef struct {
// PRNumber is the pull request number in the merger's repository.
PRNumber int
// SHA is the head commit the URI pins the pull request to.
SHA string
// Label is a short human-readable name, "owner/repo#n".
Label string
}

// resolveChange parses a change URI and rejects one that does not name a pull
// request in this merger's repository. Both failures are terminal.
func (m *githubMerger) resolveChange(uri string) (changeRef, error) {
cid, err := entitygithub.ParseChangeID(uri)
if err != nil {
return changeRef{}, fmt.Errorf("%w: invalid change URI %q: %v", merger.ErrInvalidRequest, uri, err)
}
if cid.Host != m.host || cid.Org != m.owner || cid.Repo != m.repo {
return changeRef{}, fmt.Errorf("%w: change URI %q is not in %s/%s/%s", merger.ErrInvalidRequest, uri, m.host, m.owner, m.repo)
}
return changeRef{
PRNumber: cid.PRNumber,
SHA: cid.HeadCommitSHA,
Label: fmt.Sprintf("%s#%d", cid.OwnerRepo(), cid.PRNumber),
}, nil
}
33 changes: 33 additions & 0 deletions runway/extension/merger/github/classifier.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Copyright (c) 2026 Uber Technologies, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package github

import "github.com/uber/submitqueue/platform/errs"

// Classifier implements errs.Classifier for this merger's own sentinel: an
// ErrMergePending is a retryable dependency failure, since GitHub is still
// working and a redelivery converges on the same merge. HTTP status and
// transport failures are left to platform/errs/http.
var Classifier errs.Classifier = classifier{}

type classifier struct{}

// Classify inspects a single node; the classifier-processor walks the chain.
func (classifier) Classify(err error) errs.Verdict {
if err == ErrMergePending {
return errs.InfraDependencyRetryable
}
return errs.Unknown
}
Loading
Loading