From 77054935a2782cb39b930088bdaaead1d38c3258 Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Fri, 12 Jun 2026 02:00:09 -0400 Subject: [PATCH] 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 --- cmd/submit.go | 12 ++ cmd/submit_test.go | 163 ++++++++++++++++++++++++++++ internal/github/client_interface.go | 1 + internal/github/github.go | 27 +++++ internal/github/mock_client.go | 8 ++ 5 files changed, 211 insertions(+) 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()