Skip to content

Commit 7705493

Browse files
committed
submit: disable auto-merge on existing PRs before adding to stack
When a user runs `gh stack submit` and an existing PR is discovered for a branch via `FindPRForBranch`, that PR may have auto-merge enabled. Auto-merge is incompatible with stacked PRs because the PR would merge on its own, breaking the stack's base chain. Previously, the eligibility guard for auto-merge was only in the `link` command (which blocks such PRs with an error). The `submit` command had no such check, allowing users to add auto-merge-enabled PRs to a stack by running `init` followed by `submit`. This change adds auto-merge detection and automatic disabling in `submit`'s `ensurePR` function. When an existing PR with auto-merge enabled is discovered, the CLI disables auto-merge via the `disablePullRequestAutoMerge` GraphQL mutation and warns the user. If the disable call fails, submit continues with a warning (non-fatal). The `link` command retains its stricter behavior of blocking auto-merge PRs outright, since the user explicitly chose those PRs and can fix them before retrying. Changes: internal/github/github.go: - Add DisableAutoMerge() method using the disablePullRequestAutoMerge GraphQL mutation internal/github/client_interface.go: - Add DisableAutoMerge(prID string) error to ClientOps interface internal/github/mock_client.go: - Add DisableAutoMergeFn field and mock implementation cmd/submit.go: - In ensurePR, after discovering an existing PR with auto-merge enabled, call DisableAutoMerge before proceeding. Warns on success ("Disabled auto-merge for PR #N (incompatible with stacked PRs)") and on failure ("failed to disable auto-merge"). cmd/submit_test.go: - Add TestSubmit_DisablesAutoMergeOnExistingPR: verifies auto-merge is disabled and warning is shown - Add TestSubmit_DisableAutoMergeFailure_ContinuesWithWarning: verifies submit continues even if the disable call fails - Add TestSubmit_NoAutoMerge_SkipsDisable: verifies DisableAutoMerge is not called for PRs without auto-merge
1 parent 7fc120f commit 7705493

5 files changed

Lines changed: 211 additions & 0 deletions

File tree

cmd/submit.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,18 @@ func ensurePR(cfg *config.Config, client github.ClientOps, s *stack.Stack, i int
247247
}
248248
}
249249

250+
// Disable auto-merge before adding this PR to a stack. A PR with
251+
// auto-merge enabled would merge on its own, breaking the stack.
252+
if pr.IsAutoMergeEnabled() {
253+
if err := client.DisableAutoMerge(pr.ID); err != nil {
254+
cfg.Warningf("failed to disable auto-merge for PR %s: %v",
255+
cfg.PRLink(pr.Number, pr.URL), err)
256+
} else {
257+
cfg.Warningf("Disabled auto-merge for PR %s (incompatible with stacked PRs)",
258+
cfg.PRLink(pr.Number, pr.URL))
259+
}
260+
}
261+
250262
if pr.BaseRefName != baseBranch {
251263
if s.ID != "" {
252264
// Stack API owns base relationships — can't update directly.

cmd/submit_test.go

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1800,3 +1800,166 @@ func TestSubmit_PreflightCheck_FinegrainedPAT_BailsOut(t *testing.T) {
18001800
assert.ErrorIs(t, err, ErrStacksUnavailable)
18011801
assert.Contains(t, output, "Personal access tokens are not supported by gh stack")
18021802
}
1803+
1804+
func TestSubmit_DisablesAutoMergeOnExistingPR(t *testing.T) {
1805+
s := stack.Stack{
1806+
Trunk: stack.BranchRef{Branch: "main"},
1807+
Branches: []stack.BranchRef{
1808+
{Branch: "b1"},
1809+
{Branch: "b2"},
1810+
},
1811+
}
1812+
1813+
tmpDir := t.TempDir()
1814+
writeStackFile(t, tmpDir, s)
1815+
1816+
mock := newSubmitMock(tmpDir, "b1")
1817+
mock.LogRangeFn = func(base, head string) ([]git.CommitInfo, error) {
1818+
return []git.CommitInfo{{Subject: "commit for " + head}}, nil
1819+
}
1820+
restore := git.SetOps(mock)
1821+
defer restore()
1822+
1823+
var disabledAutoMergePRIDs []string
1824+
1825+
cfg, _, errR := config.NewTestConfig()
1826+
cfg.GitHubClientOverride = &github.MockClient{
1827+
FindPRForBranchFn: func(branch string) (*github.PullRequest, error) {
1828+
switch branch {
1829+
case "b1":
1830+
return &github.PullRequest{
1831+
Number: 10, ID: "PR_10",
1832+
URL: "https://github.com/owner/repo/pull/10",
1833+
BaseRefName: "main", HeadRefName: "b1",
1834+
}, nil
1835+
case "b2":
1836+
return &github.PullRequest{
1837+
Number: 20, ID: "PR_20",
1838+
URL: "https://github.com/owner/repo/pull/20",
1839+
BaseRefName: "b1", HeadRefName: "b2",
1840+
AutoMergeRequest: &github.AutoMergeRequest{EnabledAt: "2024-01-01T00:00:00Z"},
1841+
}, nil
1842+
}
1843+
return nil, nil
1844+
},
1845+
DisableAutoMergeFn: func(prID string) error {
1846+
disabledAutoMergePRIDs = append(disabledAutoMergePRIDs, prID)
1847+
return nil
1848+
},
1849+
CreateStackFn: func(prNumbers []int) (int, error) {
1850+
return 42, nil
1851+
},
1852+
}
1853+
1854+
cmd := SubmitCmd(cfg)
1855+
cmd.SetArgs([]string{"--auto"})
1856+
cmd.SetOut(io.Discard)
1857+
cmd.SetErr(io.Discard)
1858+
err := cmd.Execute()
1859+
1860+
cfg.Err.Close()
1861+
errOut, _ := io.ReadAll(errR)
1862+
output := string(errOut)
1863+
1864+
assert.NoError(t, err)
1865+
assert.Equal(t, []string{"PR_20"}, disabledAutoMergePRIDs)
1866+
assert.Contains(t, output, "Disabled auto-merge")
1867+
assert.Contains(t, output, "incompatible with stacked PRs")
1868+
}
1869+
1870+
func TestSubmit_DisableAutoMergeFailure_ContinuesWithWarning(t *testing.T) {
1871+
s := stack.Stack{
1872+
Trunk: stack.BranchRef{Branch: "main"},
1873+
Branches: []stack.BranchRef{
1874+
{Branch: "b1"},
1875+
},
1876+
}
1877+
1878+
tmpDir := t.TempDir()
1879+
writeStackFile(t, tmpDir, s)
1880+
1881+
mock := newSubmitMock(tmpDir, "b1")
1882+
mock.LogRangeFn = func(base, head string) ([]git.CommitInfo, error) {
1883+
return []git.CommitInfo{{Subject: "commit"}}, nil
1884+
}
1885+
restore := git.SetOps(mock)
1886+
defer restore()
1887+
1888+
cfg, _, errR := config.NewTestConfig()
1889+
cfg.GitHubClientOverride = &github.MockClient{
1890+
FindPRForBranchFn: func(branch string) (*github.PullRequest, error) {
1891+
return &github.PullRequest{
1892+
Number: 10, ID: "PR_10",
1893+
URL: "https://github.com/owner/repo/pull/10",
1894+
BaseRefName: "main", HeadRefName: "b1",
1895+
AutoMergeRequest: &github.AutoMergeRequest{EnabledAt: "2024-01-01T00:00:00Z"},
1896+
}, nil
1897+
},
1898+
DisableAutoMergeFn: func(prID string) error {
1899+
return fmt.Errorf("permission denied")
1900+
},
1901+
CreateStackFn: func(prNumbers []int) (int, error) {
1902+
return 42, nil
1903+
},
1904+
}
1905+
1906+
cmd := SubmitCmd(cfg)
1907+
cmd.SetArgs([]string{"--auto"})
1908+
cmd.SetOut(io.Discard)
1909+
cmd.SetErr(io.Discard)
1910+
err := cmd.Execute()
1911+
1912+
cfg.Err.Close()
1913+
errOut, _ := io.ReadAll(errR)
1914+
output := string(errOut)
1915+
1916+
// Submit should succeed even if disable-auto-merge fails
1917+
assert.NoError(t, err)
1918+
assert.Contains(t, output, "failed to disable auto-merge")
1919+
assert.Contains(t, output, "permission denied")
1920+
}
1921+
1922+
func TestSubmit_NoAutoMerge_SkipsDisable(t *testing.T) {
1923+
s := stack.Stack{
1924+
Trunk: stack.BranchRef{Branch: "main"},
1925+
Branches: []stack.BranchRef{
1926+
{Branch: "b1"},
1927+
},
1928+
}
1929+
1930+
tmpDir := t.TempDir()
1931+
writeStackFile(t, tmpDir, s)
1932+
1933+
mock := newSubmitMock(tmpDir, "b1")
1934+
mock.LogRangeFn = func(base, head string) ([]git.CommitInfo, error) {
1935+
return []git.CommitInfo{{Subject: "commit"}}, nil
1936+
}
1937+
restore := git.SetOps(mock)
1938+
defer restore()
1939+
1940+
cfg, _, _ := config.NewTestConfig()
1941+
cfg.GitHubClientOverride = &github.MockClient{
1942+
FindPRForBranchFn: func(branch string) (*github.PullRequest, error) {
1943+
return &github.PullRequest{
1944+
Number: 10, ID: "PR_10",
1945+
URL: "https://github.com/owner/repo/pull/10",
1946+
BaseRefName: "main", HeadRefName: "b1",
1947+
}, nil
1948+
},
1949+
DisableAutoMergeFn: func(prID string) error {
1950+
t.Fatal("DisableAutoMerge should not be called when auto-merge is not enabled")
1951+
return nil
1952+
},
1953+
CreateStackFn: func(prNumbers []int) (int, error) {
1954+
return 42, nil
1955+
},
1956+
}
1957+
1958+
cmd := SubmitCmd(cfg)
1959+
cmd.SetArgs([]string{"--auto"})
1960+
cmd.SetOut(io.Discard)
1961+
cmd.SetErr(io.Discard)
1962+
err := cmd.Execute()
1963+
1964+
assert.NoError(t, err)
1965+
}

internal/github/client_interface.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ type ClientOps interface {
1010
CreatePR(base, head, title, body string, draft bool) (*PullRequest, error)
1111
UpdatePRBase(number int, base string) error
1212
MarkPRReadyForReview(prID string) error
13+
DisableAutoMerge(prID string) error
1314
ListStacks() ([]RemoteStack, error)
1415
CreateStack(prNumbers []int) (int, error)
1516
UpdateStack(stackID string, prNumbers []int) error

internal/github/github.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,33 @@ func (c *Client) MarkPRReadyForReview(prID string) error {
228228
return nil
229229
}
230230

231+
// DisableAutoMerge disables auto-merge on a pull request.
232+
func (c *Client) DisableAutoMerge(prID string) error {
233+
var mutation struct {
234+
DisablePullRequestAutoMerge struct {
235+
PullRequest struct {
236+
ID string
237+
}
238+
} `graphql:"disablePullRequestAutoMerge(input: $input)"`
239+
}
240+
241+
type DisablePullRequestAutoMergeInput struct {
242+
PullRequestID string `json:"pullRequestId"`
243+
}
244+
245+
variables := map[string]interface{}{
246+
"input": DisablePullRequestAutoMergeInput{
247+
PullRequestID: prID,
248+
},
249+
}
250+
251+
if err := c.gql.Mutate("DisablePullRequestAutoMerge", &mutation, variables); err != nil {
252+
return fmt.Errorf("disabling auto-merge: %w", err)
253+
}
254+
255+
return nil
256+
}
257+
231258
func (c *Client) repositoryID() (string, error) {
232259
var query struct {
233260
Repository struct {

internal/github/mock_client.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ type MockClient struct {
1010
CreatePRFn func(string, string, string, string, bool) (*PullRequest, error)
1111
UpdatePRBaseFn func(int, string) error
1212
MarkPRReadyForReviewFn func(string) error
13+
DisableAutoMergeFn func(string) error
1314
ListStacksFn func() ([]RemoteStack, error)
1415
CreateStackFn func([]int) (int, error)
1516
UpdateStackFn func(string, []int) error
@@ -61,6 +62,13 @@ func (m *MockClient) MarkPRReadyForReview(prID string) error {
6162
return nil
6263
}
6364

65+
func (m *MockClient) DisableAutoMerge(prID string) error {
66+
if m.DisableAutoMergeFn != nil {
67+
return m.DisableAutoMergeFn(prID)
68+
}
69+
return nil
70+
}
71+
6472
func (m *MockClient) ListStacks() ([]RemoteStack, error) {
6573
if m.ListStacksFn != nil {
6674
return m.ListStacksFn()

0 commit comments

Comments
 (0)