Skip to content

Commit 4d83e3c

Browse files
committed
warn merge queue unsupported
1 parent 7b769ad commit 4d83e3c

6 files changed

Lines changed: 157 additions & 0 deletions

File tree

cmd/merge.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,18 @@ func runMerge(cfg *config.Config, opts *mergeOptions, args []string) error {
140140

141141
base := remoteStack.Base.Ref
142142

143+
// Merge-queue and admin-bypass policy for the stack's base branch. The async
144+
// stack merge cannot use a merge queue, so bail out early (before any
145+
// prompting) when the base requires one.
146+
policy, err := client.BaseBranchPolicy(base)
147+
if err != nil {
148+
cfg.Errorf("failed to check base branch merge settings: %s", err)
149+
return ErrAPIFailure
150+
}
151+
if policy.RequiresMergeQueue {
152+
return explainMergeQueueUnsupported(cfg, base)
153+
}
154+
143155
if cfg.IsInteractive() && !opts.yes {
144156
return runMergeInteractive(cfg, client, remoteStack.Number, base, candidates, allowed, mergeCfg.DefaultMethod, method, preselectIndex, opts)
145157
}
@@ -476,6 +488,15 @@ func warnAsyncMergeUnavailable(cfg *config.Config) {
476488
cfg.Warningf("Async stack merge is not available for this repository")
477489
}
478490

491+
// explainMergeQueueUnsupported reports that the stack's base branch merges
492+
// through a merge queue, which the async stack merge cannot use, and points the
493+
// user to the web UI.
494+
func explainMergeQueueUnsupported(cfg *config.Config, base string) error {
495+
cfg.Errorf("the base branch %q requires a merge queue, which \"gh stack merge\" does not support", base)
496+
cfg.Printf("Merge this stack using `%q` or from the GitHub web UI instead.", "gh pr merge")
497+
return ErrSilent
498+
}
499+
479500
// mergeFailureExit maps a merge failure message to an exit code: rebase/merge
480501
// conflicts get ErrConflict, everything else ErrAPIFailure.
481502
func mergeFailureExit(message string) error {

cmd/merge_test.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,30 @@ func TestRunMerge_AlreadyMergedOnSubmit(t *testing.T) {
347347
assert.Contains(t, output, "Merged #1, #2 into main")
348348
}
349349

350+
func TestRunMerge_MergeQueueRequired(t *testing.T) {
351+
cfg, outR, errR := config.NewTestConfig()
352+
cfg.GitHubClientOverride = &github.MockClient{
353+
GetStackFn: func(n int) (*github.RemoteStack, error) {
354+
return remoteStack(7, "main", openStackPR(1, "b1"), openStackPR(2, "b2")), nil
355+
},
356+
BaseBranchPolicyFn: func(base string) (*github.BaseBranchPolicy, error) {
357+
assert.Equal(t, "main", base)
358+
return &github.BaseBranchPolicy{RequiresMergeQueue: true}, nil
359+
},
360+
MergeStackAsyncFn: func(pr int, method string) (*github.AsyncMergeResult, error) {
361+
t.Fatal("merge must not be attempted when the base requires a merge queue")
362+
return nil, nil
363+
},
364+
}
365+
366+
err := runMerge(cfg, fastOptions(), []string{"7"})
367+
output := collectOutput(cfg, outR, errR)
368+
369+
assert.ErrorIs(t, err, ErrSilent)
370+
assert.Contains(t, output, "merge queue")
371+
assert.Contains(t, output, "web UI")
372+
}
373+
350374
func TestRunMerge_AsyncMergeUnavailable(t *testing.T) {
351375
cfg, outR, errR := config.NewTestConfig()
352376
cfg.GitHubClientOverride = &github.MockClient{

internal/github/client_interface.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ type ClientOps interface {
2121
MergeStackAsync(prNumber int, method string) (*AsyncMergeResult, error)
2222
GetAsyncMergeResult(prNumber int, uuid string) (*AsyncMergeResult, error)
2323
PRTitles(numbers []int) (map[int]string, error)
24+
BaseBranchPolicy(baseRef string) (*BaseBranchPolicy, error)
2425
}
2526

2627
// Compile-time check that Client satisfies ClientOps.

internal/github/merge_async.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,54 @@ func (c *Client) RepoMergeConfig() (*RepoMergeConfig, error) {
121121
}, nil
122122
}
123123

124+
// BaseBranchPolicy describes merge-relevant policy on a stack's base branch.
125+
type BaseBranchPolicy struct {
126+
// RequiresMergeQueue reports that the base branch merges through a merge
127+
// queue, so a direct async stack merge is not possible.
128+
RequiresMergeQueue bool
129+
}
130+
131+
// BaseBranchPolicy reports whether the given base branch requires a merge queue,
132+
// which the async stack merge cannot use.
133+
func (c *Client) BaseBranchPolicy(baseRef string) (*BaseBranchPolicy, error) {
134+
var query struct {
135+
Repository struct {
136+
MergeQueue *struct {
137+
ID string `graphql:"id"`
138+
} `graphql:"mergeQueue(branch: $branch)"`
139+
Ref *struct {
140+
Rules struct {
141+
Nodes []struct {
142+
Type string `graphql:"type"`
143+
} `graphql:"nodes"`
144+
} `graphql:"rules(first: 50)"`
145+
} `graphql:"ref(qualifiedName: $qualified)"`
146+
} `graphql:"repository(owner: $owner, name: $name)"`
147+
}
148+
149+
variables := map[string]interface{}{
150+
"owner": graphql.String(c.owner),
151+
"name": graphql.String(c.repo),
152+
"branch": graphql.String(baseRef),
153+
"qualified": graphql.String("refs/heads/" + baseRef),
154+
}
155+
156+
if err := c.gql.Query("BaseBranchPolicy", &query, variables); err != nil {
157+
return nil, fmt.Errorf("querying base branch policy: %w", err)
158+
}
159+
160+
r := query.Repository
161+
policy := &BaseBranchPolicy{RequiresMergeQueue: r.MergeQueue != nil}
162+
if r.Ref != nil {
163+
for _, node := range r.Ref.Rules.Nodes {
164+
if node.Type == "MERGE_QUEUE" {
165+
policy.RequiresMergeQueue = true
166+
}
167+
}
168+
}
169+
return policy, nil
170+
}
171+
124172
// MergeStackAsync requests an asynchronous merge of the given pull request. For
125173
// a stacked PR this merges all members of the stack up to and including
126174
// prNumber. A blank method lets the server apply its default.

internal/github/merge_async_test.go

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,3 +169,58 @@ func TestMergeStackAsync_OmitsEmptyMethod(t *testing.T) {
169169
_, hasMethod := parsed["merge_method"]
170170
assert.False(t, hasMethod, "merge_method should be omitted when empty")
171171
}
172+
173+
// testPolicyClient builds a Client whose GraphQL client is backed by a stub
174+
// transport returning the given response body.
175+
func testPolicyClient(t *testing.T, graphqlResp string) *Client {
176+
t.Helper()
177+
rt := roundTripFunc(func(r *http.Request) (*http.Response, error) {
178+
return &http.Response{
179+
StatusCode: http.StatusOK,
180+
Body: io.NopCloser(strings.NewReader(graphqlResp)),
181+
Header: http.Header{"Content-Type": []string{"application/json"}},
182+
Request: r,
183+
}, nil
184+
})
185+
gql, err := api.NewGraphQLClient(api.ClientOptions{Host: "github.com", AuthToken: "x", Transport: rt})
186+
require.NoError(t, err)
187+
return &Client{gql: gql, owner: "o", repo: "r"}
188+
}
189+
190+
func TestBaseBranchPolicy(t *testing.T) {
191+
tests := []struct {
192+
name string
193+
body string
194+
wantQueue bool
195+
}{
196+
{
197+
name: "no merge queue",
198+
body: `{"data":{"repository":{"mergeQueue":null,"ref":{"rules":{"nodes":[]}}}}}`,
199+
},
200+
{
201+
name: "merge queue via mergeQueue field",
202+
body: `{"data":{"repository":{"mergeQueue":{"id":"MQ"},"ref":{"rules":{"nodes":[]}}}}}`,
203+
wantQueue: true,
204+
},
205+
{
206+
name: "merge queue via ruleset type",
207+
body: `{"data":{"repository":{"mergeQueue":null,"ref":{"rules":{"nodes":[{"type":"MERGE_QUEUE"}]}}}}}`,
208+
wantQueue: true,
209+
},
210+
{
211+
name: "other rules, no merge queue",
212+
body: `{"data":{"repository":{"mergeQueue":null,"ref":{"rules":{"nodes":[{"type":"PULL_REQUEST"}]}}}}}`,
213+
},
214+
{
215+
name: "null ref",
216+
body: `{"data":{"repository":{"mergeQueue":null,"ref":null}}}`,
217+
},
218+
}
219+
for _, tt := range tests {
220+
t.Run(tt.name, func(t *testing.T) {
221+
policy, err := testPolicyClient(t, tt.body).BaseBranchPolicy("main")
222+
require.NoError(t, err)
223+
assert.Equal(t, tt.wantQueue, policy.RequiresMergeQueue, "RequiresMergeQueue")
224+
})
225+
}
226+
}

internal/github/mock_client.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ type MockClient struct {
2121
MergeStackAsyncFn func(int, string) (*AsyncMergeResult, error)
2222
GetAsyncMergeResultFn func(int, string) (*AsyncMergeResult, error)
2323
PRTitlesFn func([]int) (map[int]string, error)
24+
BaseBranchPolicyFn func(string) (*BaseBranchPolicy, error)
2425
}
2526

2627
// Compile-time check that MockClient satisfies ClientOps.
@@ -162,3 +163,10 @@ func (m *MockClient) PRTitles(numbers []int) (map[int]string, error) {
162163
}
163164
return map[int]string{}, nil
164165
}
166+
167+
func (m *MockClient) BaseBranchPolicy(baseRef string) (*BaseBranchPolicy, error) {
168+
if m.BaseBranchPolicyFn != nil {
169+
return m.BaseBranchPolicyFn(baseRef)
170+
}
171+
return &BaseBranchPolicy{}, nil
172+
}

0 commit comments

Comments
 (0)