diff --git a/cmd/submit.go b/cmd/submit.go index 04d534f..1824bcf 100644 --- a/cmd/submit.go +++ b/cmd/submit.go @@ -247,6 +247,18 @@ func ensurePR(cfg *config.Config, client github.ClientOps, s *stack.Stack, i int } } + // Disable auto-merge before adding this PR to a stack. A PR with + // auto-merge enabled would merge on its own, breaking the stack. + if pr.IsAutoMergeEnabled() { + if err := client.DisableAutoMerge(pr.ID); err != nil { + cfg.Warningf("failed to disable auto-merge for PR %s: %v", + cfg.PRLink(pr.Number, pr.URL), err) + } else { + cfg.Warningf("Disabled auto-merge for PR %s (incompatible with stacked PRs)", + cfg.PRLink(pr.Number, pr.URL)) + } + } + if pr.BaseRefName != baseBranch { if s.ID != "" { // Stack API owns base relationships — can't update directly. diff --git a/cmd/submit_test.go b/cmd/submit_test.go index c35dcd5..ae010d9 100644 --- a/cmd/submit_test.go +++ b/cmd/submit_test.go @@ -1800,3 +1800,166 @@ func TestSubmit_PreflightCheck_FinegrainedPAT_BailsOut(t *testing.T) { assert.ErrorIs(t, err, ErrStacksUnavailable) assert.Contains(t, output, "Personal access tokens are not supported by gh stack") } + +func TestSubmit_DisablesAutoMergeOnExistingPR(t *testing.T) { + s := stack.Stack{ + Trunk: stack.BranchRef{Branch: "main"}, + Branches: []stack.BranchRef{ + {Branch: "b1"}, + {Branch: "b2"}, + }, + } + + tmpDir := t.TempDir() + writeStackFile(t, tmpDir, s) + + mock := newSubmitMock(tmpDir, "b1") + mock.LogRangeFn = func(base, head string) ([]git.CommitInfo, error) { + return []git.CommitInfo{{Subject: "commit for " + head}}, nil + } + restore := git.SetOps(mock) + defer restore() + + var disabledAutoMergePRIDs []string + + cfg, _, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRForBranchFn: func(branch string) (*github.PullRequest, error) { + switch branch { + case "b1": + return &github.PullRequest{ + Number: 10, ID: "PR_10", + URL: "https://github.com/owner/repo/pull/10", + BaseRefName: "main", HeadRefName: "b1", + }, nil + case "b2": + return &github.PullRequest{ + Number: 20, ID: "PR_20", + URL: "https://github.com/owner/repo/pull/20", + BaseRefName: "b1", HeadRefName: "b2", + AutoMergeRequest: &github.AutoMergeRequest{EnabledAt: "2024-01-01T00:00:00Z"}, + }, nil + } + return nil, nil + }, + DisableAutoMergeFn: func(prID string) error { + disabledAutoMergePRIDs = append(disabledAutoMergePRIDs, prID) + return nil + }, + CreateStackFn: func(prNumbers []int) (int, error) { + return 42, nil + }, + } + + cmd := SubmitCmd(cfg) + cmd.SetArgs([]string{"--auto"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + + cfg.Err.Close() + errOut, _ := io.ReadAll(errR) + output := string(errOut) + + assert.NoError(t, err) + assert.Equal(t, []string{"PR_20"}, disabledAutoMergePRIDs) + assert.Contains(t, output, "Disabled auto-merge") + assert.Contains(t, output, "incompatible with stacked PRs") +} + +func TestSubmit_DisableAutoMergeFailure_ContinuesWithWarning(t *testing.T) { + s := stack.Stack{ + Trunk: stack.BranchRef{Branch: "main"}, + Branches: []stack.BranchRef{ + {Branch: "b1"}, + }, + } + + tmpDir := t.TempDir() + writeStackFile(t, tmpDir, s) + + mock := newSubmitMock(tmpDir, "b1") + mock.LogRangeFn = func(base, head string) ([]git.CommitInfo, error) { + return []git.CommitInfo{{Subject: "commit"}}, nil + } + restore := git.SetOps(mock) + defer restore() + + cfg, _, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRForBranchFn: func(branch string) (*github.PullRequest, error) { + return &github.PullRequest{ + Number: 10, ID: "PR_10", + URL: "https://github.com/owner/repo/pull/10", + BaseRefName: "main", HeadRefName: "b1", + AutoMergeRequest: &github.AutoMergeRequest{EnabledAt: "2024-01-01T00:00:00Z"}, + }, nil + }, + DisableAutoMergeFn: func(prID string) error { + return fmt.Errorf("permission denied") + }, + CreateStackFn: func(prNumbers []int) (int, error) { + return 42, nil + }, + } + + cmd := SubmitCmd(cfg) + cmd.SetArgs([]string{"--auto"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + + cfg.Err.Close() + errOut, _ := io.ReadAll(errR) + output := string(errOut) + + // Submit should succeed even if disable-auto-merge fails + assert.NoError(t, err) + assert.Contains(t, output, "failed to disable auto-merge") + assert.Contains(t, output, "permission denied") +} + +func TestSubmit_NoAutoMerge_SkipsDisable(t *testing.T) { + s := stack.Stack{ + Trunk: stack.BranchRef{Branch: "main"}, + Branches: []stack.BranchRef{ + {Branch: "b1"}, + }, + } + + tmpDir := t.TempDir() + writeStackFile(t, tmpDir, s) + + mock := newSubmitMock(tmpDir, "b1") + mock.LogRangeFn = func(base, head string) ([]git.CommitInfo, error) { + return []git.CommitInfo{{Subject: "commit"}}, nil + } + restore := git.SetOps(mock) + defer restore() + + cfg, _, _ := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRForBranchFn: func(branch string) (*github.PullRequest, error) { + return &github.PullRequest{ + Number: 10, ID: "PR_10", + URL: "https://github.com/owner/repo/pull/10", + BaseRefName: "main", HeadRefName: "b1", + }, nil + }, + DisableAutoMergeFn: func(prID string) error { + t.Fatal("DisableAutoMerge should not be called when auto-merge is not enabled") + return nil + }, + CreateStackFn: func(prNumbers []int) (int, error) { + return 42, nil + }, + } + + cmd := SubmitCmd(cfg) + cmd.SetArgs([]string{"--auto"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + + assert.NoError(t, err) +} diff --git a/internal/github/client_interface.go b/internal/github/client_interface.go index daeb512..7ff9bdb 100644 --- a/internal/github/client_interface.go +++ b/internal/github/client_interface.go @@ -10,6 +10,7 @@ type ClientOps interface { CreatePR(base, head, title, body string, draft bool) (*PullRequest, error) UpdatePRBase(number int, base string) error MarkPRReadyForReview(prID string) error + DisableAutoMerge(prID string) error ListStacks() ([]RemoteStack, error) CreateStack(prNumbers []int) (int, error) UpdateStack(stackID string, prNumbers []int) error diff --git a/internal/github/github.go b/internal/github/github.go index 7e3784d..6a5a79b 100644 --- a/internal/github/github.go +++ b/internal/github/github.go @@ -228,6 +228,33 @@ func (c *Client) MarkPRReadyForReview(prID string) error { return nil } +// DisableAutoMerge disables auto-merge on a pull request. +func (c *Client) DisableAutoMerge(prID string) error { + var mutation struct { + DisablePullRequestAutoMerge struct { + PullRequest struct { + ID string + } + } `graphql:"disablePullRequestAutoMerge(input: $input)"` + } + + type DisablePullRequestAutoMergeInput struct { + PullRequestID string `json:"pullRequestId"` + } + + variables := map[string]interface{}{ + "input": DisablePullRequestAutoMergeInput{ + PullRequestID: prID, + }, + } + + if err := c.gql.Mutate("DisablePullRequestAutoMerge", &mutation, variables); err != nil { + return fmt.Errorf("disabling auto-merge: %w", err) + } + + return nil +} + func (c *Client) repositoryID() (string, error) { var query struct { Repository struct { diff --git a/internal/github/mock_client.go b/internal/github/mock_client.go index ed450dd..5bff2b8 100644 --- a/internal/github/mock_client.go +++ b/internal/github/mock_client.go @@ -10,6 +10,7 @@ type MockClient struct { CreatePRFn func(string, string, string, string, bool) (*PullRequest, error) UpdatePRBaseFn func(int, string) error MarkPRReadyForReviewFn func(string) error + DisableAutoMergeFn func(string) error ListStacksFn func() ([]RemoteStack, error) CreateStackFn func([]int) (int, error) UpdateStackFn func(string, []int) error @@ -61,6 +62,13 @@ func (m *MockClient) MarkPRReadyForReview(prID string) error { return nil } +func (m *MockClient) DisableAutoMerge(prID string) error { + if m.DisableAutoMergeFn != nil { + return m.DisableAutoMergeFn(prID) + } + return nil +} + func (m *MockClient) ListStacks() ([]RemoteStack, error) { if m.ListStacksFn != nil { return m.ListStacksFn()