Skip to content

Commit 0779491

Browse files
skarimskarim
committed
preflight check for stacked prs before submit
1 parent 1612fa5 commit 0779491

File tree

6 files changed

+272
-10
lines changed

6 files changed

+272
-10
lines changed

‎cmd/submit.go‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,32 @@ func runSubmit(cfg *config.Config, opts *submitOptions) error {
7777
return ErrAPIFailure
7878
}
7979

80+
// Verify that the repository has stacked PRs enabled.
81+
stacksAvailable := s.ID != ""
82+
if s.ID == "" {
83+
if _, err := client.ListStacks(); err != nil {
84+
cfg.Warningf("Stacked PRs are not enabled for this repository")
85+
if cfg.IsInteractive() {
86+
p := prompter.New(cfg.In, cfg.Out, cfg.Err)
87+
proceed, promptErr := p.Confirm("Would you still like to create regular PRs?", false)
88+
if promptErr != nil {
89+
if isInterruptError(promptErr) {
90+
printInterrupt(cfg)
91+
return ErrSilent
92+
}
93+
return ErrStacksUnavailable
94+
}
95+
if !proceed {
96+
return ErrStacksUnavailable
97+
}
98+
} else {
99+
return ErrStacksUnavailable
100+
}
101+
} else {
102+
stacksAvailable = true
103+
}
104+
}
105+
80106
// Sync PR state to detect merged/queued PRs before pushing.
81107
syncStackPRs(cfg, s)
82108

@@ -194,7 +220,9 @@ func runSubmit(cfg *config.Config, opts *submitOptions) error {
194220
}
195221

196222
// Create or update the stack on GitHub
197-
syncStack(cfg, client, s)
223+
if stacksAvailable {
224+
syncStack(cfg, client, s)
225+
}
198226

199227
// Update base commit hashes and sync PR state
200228
updateBaseSHAs(s)

‎cmd/submit_test.go‎

Lines changed: 226 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"fmt"
55
"io"
66
"net/url"
7+
"os"
78
"testing"
89

910
"github.com/cli/go-gh/v2/pkg/api"
@@ -829,3 +830,228 @@ func TestSubmit_CreatesMissingPRsAndUpdatesExisting(t *testing.T) {
829830
// Stack should be created with all 3 PRs
830831
assert.Contains(t, output, "Stack created on GitHub with 3 PRs")
831832
}
833+
834+
func TestSubmit_PreflightCheck_404_BailsOut(t *testing.T) {
835+
s := stack.Stack{
836+
// No ID — this is a new stack, so the pre-flight check will run.
837+
Trunk: stack.BranchRef{Branch: "main"},
838+
Branches: []stack.BranchRef{
839+
{Branch: "b1"},
840+
{Branch: "b2"},
841+
},
842+
}
843+
844+
tmpDir := t.TempDir()
845+
writeStackFile(t, tmpDir, s)
846+
847+
pushed := false
848+
mock := newSubmitMock(tmpDir, "b1")
849+
mock.PushFn = func(string, []string, bool, bool) error {
850+
pushed = true
851+
return nil
852+
}
853+
restore := git.SetOps(mock)
854+
defer restore()
855+
856+
// Non-interactive config — should bail out immediately.
857+
cfg, _, errR := config.NewTestConfig()
858+
cfg.GitHubClientOverride = &github.MockClient{
859+
ListStacksFn: func() ([]github.RemoteStack, error) {
860+
return nil, &api.HTTPError{StatusCode: 404, Message: "Not Found"}
861+
},
862+
}
863+
864+
cmd := SubmitCmd(cfg)
865+
cmd.SetArgs([]string{"--auto"})
866+
cmd.SetOut(io.Discard)
867+
cmd.SetErr(io.Discard)
868+
err := cmd.Execute()
869+
870+
cfg.Err.Close()
871+
errOut, _ := io.ReadAll(errR)
872+
output := string(errOut)
873+
874+
assert.ErrorIs(t, err, ErrStacksUnavailable)
875+
assert.Contains(t, output, "Stacked PRs are not enabled for this repository")
876+
assert.False(t, pushed, "should not push when stacks are unavailable")
877+
}
878+
879+
func TestSubmit_PreflightCheck_404_Interactive_UserDeclinesAborts(t *testing.T) {
880+
s := stack.Stack{
881+
Trunk: stack.BranchRef{Branch: "main"},
882+
Branches: []stack.BranchRef{
883+
{Branch: "b1"},
884+
{Branch: "b2"},
885+
},
886+
}
887+
888+
tmpDir := t.TempDir()
889+
writeStackFile(t, tmpDir, s)
890+
891+
pushed := false
892+
mock := newSubmitMock(tmpDir, "b1")
893+
mock.PushFn = func(string, []string, bool, bool) error {
894+
pushed = true
895+
return nil
896+
}
897+
restore := git.SetOps(mock)
898+
defer restore()
899+
900+
// Force interactive mode; survey will fail on the pipe,
901+
// which is treated as a decline — same as user saying "no".
902+
inR, inW, _ := os.Pipe()
903+
inW.Close()
904+
905+
cfg, _, errR := config.NewTestConfig()
906+
cfg.In = inR
907+
cfg.ForceInteractive = true
908+
cfg.GitHubClientOverride = &github.MockClient{
909+
ListStacksFn: func() ([]github.RemoteStack, error) {
910+
return nil, &api.HTTPError{StatusCode: 404, Message: "Not Found"}
911+
},
912+
}
913+
914+
cmd := SubmitCmd(cfg)
915+
cmd.SetArgs([]string{"--auto"})
916+
cmd.SetOut(io.Discard)
917+
cmd.SetErr(io.Discard)
918+
err := cmd.Execute()
919+
920+
cfg.Err.Close()
921+
errOut, _ := io.ReadAll(errR)
922+
output := string(errOut)
923+
924+
assert.ErrorIs(t, err, ErrStacksUnavailable)
925+
assert.Contains(t, output, "Stacked PRs are not enabled for this repository")
926+
assert.False(t, pushed, "should not push when user declines")
927+
}
928+
929+
func TestSyncStack_SkippedWhenStacksUnavailable(t *testing.T) {
930+
// Verify that syncStack is not called when stacksAvailable is false.
931+
// This is the core behavior enabling unstacked PR creation.
932+
s := &stack.Stack{
933+
Trunk: stack.BranchRef{Branch: "main"},
934+
Branches: []stack.BranchRef{
935+
{Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 10}},
936+
{Branch: "b2", PullRequest: &stack.PullRequestRef{Number: 11}},
937+
},
938+
}
939+
940+
createCalled := false
941+
mock := &github.MockClient{
942+
CreateStackFn: func(prNumbers []int) (int, error) {
943+
createCalled = true
944+
return 42, nil
945+
},
946+
}
947+
948+
cfg, _, errR := config.NewTestConfig()
949+
950+
// When stacksAvailable=true, syncStack should be called.
951+
syncStack(cfg, mock, s)
952+
assert.True(t, createCalled, "syncStack should call CreateStack when invoked")
953+
954+
// When stacksAvailable=false, the caller (runSubmit) skips syncStack
955+
// entirely — verified by the submit_test integration tests above.
956+
// Here we just confirm the contract: if syncStack is NOT called,
957+
// CreateStack is NOT called.
958+
createCalled = false
959+
// (not calling syncStack)
960+
assert.False(t, createCalled, "CreateStack should not be called when syncStack is skipped")
961+
962+
cfg.Err.Close()
963+
_, _ = io.ReadAll(errR)
964+
}
965+
966+
func TestSubmit_PreflightCheck_EmptyList_Proceeds(t *testing.T) {
967+
s := stack.Stack{
968+
Trunk: stack.BranchRef{Branch: "main"},
969+
Branches: []stack.BranchRef{
970+
{Branch: "b1"},
971+
{Branch: "b2"},
972+
},
973+
}
974+
975+
tmpDir := t.TempDir()
976+
writeStackFile(t, tmpDir, s)
977+
978+
pushed := false
979+
mock := newSubmitMock(tmpDir, "b1")
980+
mock.PushFn = func(string, []string, bool, bool) error {
981+
pushed = true
982+
return nil
983+
}
984+
mock.LogRangeFn = func(base, head string) ([]git.CommitInfo, error) {
985+
return []git.CommitInfo{{Subject: "commit for " + head}}, nil
986+
}
987+
restore := git.SetOps(mock)
988+
defer restore()
989+
990+
cfg, _, errR := config.NewTestConfig()
991+
cfg.GitHubClientOverride = &github.MockClient{
992+
ListStacksFn: func() ([]github.RemoteStack, error) {
993+
return []github.RemoteStack{}, nil
994+
},
995+
FindPRForBranchFn: func(string) (*github.PullRequest, error) { return nil, nil },
996+
CreatePRFn: func(base, head, title, body string, draft bool) (*github.PullRequest, error) {
997+
return &github.PullRequest{Number: 1, ID: "PR_1", URL: "https://github.com/o/r/pull/1"}, nil
998+
},
999+
CreateStackFn: func([]int) (int, error) { return 99, nil },
1000+
}
1001+
1002+
cmd := SubmitCmd(cfg)
1003+
cmd.SetArgs([]string{"--auto"})
1004+
cmd.SetOut(io.Discard)
1005+
cmd.SetErr(io.Discard)
1006+
err := cmd.Execute()
1007+
1008+
cfg.Err.Close()
1009+
_, _ = io.ReadAll(errR)
1010+
1011+
assert.NoError(t, err)
1012+
assert.True(t, pushed, "should proceed with push when ListStacks succeeds")
1013+
}
1014+
1015+
func TestSubmit_PreflightCheck_SkippedWhenStackIDSet(t *testing.T) {
1016+
s := stack.Stack{
1017+
ID: "42", // Existing stack — pre-flight check should be skipped.
1018+
Trunk: stack.BranchRef{Branch: "main"},
1019+
Branches: []stack.BranchRef{
1020+
{Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 10}},
1021+
{Branch: "b2", PullRequest: &stack.PullRequestRef{Number: 11}},
1022+
},
1023+
}
1024+
1025+
tmpDir := t.TempDir()
1026+
writeStackFile(t, tmpDir, s)
1027+
1028+
listStacksCalled := false
1029+
mock := newSubmitMock(tmpDir, "b1")
1030+
mock.PushFn = func(string, []string, bool, bool) error { return nil }
1031+
restore := git.SetOps(mock)
1032+
defer restore()
1033+
1034+
cfg, _, errR := config.NewTestConfig()
1035+
cfg.GitHubClientOverride = &github.MockClient{
1036+
ListStacksFn: func() ([]github.RemoteStack, error) {
1037+
listStacksCalled = true
1038+
return nil, &api.HTTPError{StatusCode: 404, Message: "Not Found"}
1039+
},
1040+
FindPRForBranchFn: func(string) (*github.PullRequest, error) {
1041+
return &github.PullRequest{Number: 10, URL: "https://github.com/o/r/pull/10"}, nil
1042+
},
1043+
UpdateStackFn: func(string, []int) error { return nil },
1044+
}
1045+
1046+
cmd := SubmitCmd(cfg)
1047+
cmd.SetArgs([]string{"--auto"})
1048+
cmd.SetOut(io.Discard)
1049+
cmd.SetErr(io.Discard)
1050+
err := cmd.Execute()
1051+
1052+
cfg.Err.Close()
1053+
_, _ = io.ReadAll(errR)
1054+
1055+
assert.NoError(t, err)
1056+
assert.False(t, listStacksCalled, "ListStacks should not be called when stack ID already exists")
1057+
}

‎cmd/utils.go‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,14 @@ var ErrSilent = &ExitError{Code: 1}
2020

2121
// Typed exit errors for programmatic detection by scripts and agents.
2222
var (
23-
ErrNotInStack = &ExitError{Code: 2} // branch/stack not found
24-
ErrConflict = &ExitError{Code: 3} // rebase conflict
25-
ErrAPIFailure = &ExitError{Code: 4} // GitHub API error
26-
ErrInvalidArgs = &ExitError{Code: 5} // invalid arguments or flags
27-
ErrDisambiguate = &ExitError{Code: 6} // multiple stacks/remotes, can't auto-select
28-
ErrRebaseActive = &ExitError{Code: 7} // rebase already in progress
29-
ErrLockFailed = &ExitError{Code: 8} // could not acquire stack file lock
23+
ErrNotInStack = &ExitError{Code: 2} // branch/stack not found
24+
ErrConflict = &ExitError{Code: 3} // rebase conflict
25+
ErrAPIFailure = &ExitError{Code: 4} // GitHub API error
26+
ErrInvalidArgs = &ExitError{Code: 5} // invalid arguments or flags
27+
ErrDisambiguate = &ExitError{Code: 6} // multiple stacks/remotes, can't auto-select
28+
ErrRebaseActive = &ExitError{Code: 7} // rebase already in progress
29+
ErrLockFailed = &ExitError{Code: 8} // could not acquire stack file lock
30+
ErrStacksUnavailable = &ExitError{Code: 9} // stacked PRs not available for this repository
3031
)
3132

3233
// ExitError is returned by commands to indicate a specific exit code.

‎docs/src/content/docs/reference/cli.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -441,3 +441,4 @@ gh stack feedback "Support for reordering branches"
441441
| 6 | Disambiguation required (branch belongs to multiple stacks) |
442442
| 7 | Rebase already in progress |
443443
| 8 | Stack is locked by another process |
444+
| 9 | Stacked PRs not enabled for this repository |

‎internal/config/config.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@ type Config struct {
3030
// GitHubClientOverride, when non-nil, is returned by GitHubClient()
3131
// instead of creating a real client. Used in tests to inject a MockClient.
3232
GitHubClientOverride ghapi.ClientOps
33+
34+
// ForceInteractive, when true, makes IsInteractive() return true
35+
// regardless of the terminal state. Used in tests.
36+
ForceInteractive bool
3337
}
3438

3539
// New creates a new Config with terminal-aware output and color support.
@@ -106,7 +110,7 @@ func (c *Config) PRLink(number int, url string) string {
106110
}
107111

108112
func (c *Config) IsInteractive() bool {
109-
return c.Terminal.IsTerminalOutput()
113+
return c.ForceInteractive || c.Terminal.IsTerminalOutput()
110114
}
111115

112116
func (c *Config) Repo() (repository.Repository, error) {

‎skills/gh-stack/SKILL.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ description: >
77
branch chains, or incremental code review workflows.
88
metadata:
99
author: github
10-
version: "0.0.1"
10+
version: "0.0.2"
1111
---
1212

1313
# gh-stack
@@ -539,6 +539,7 @@ gh stack submit --auto --draft
539539
- Pushes all active (non-merged) branches atomically (`--force-with-lease --atomic`)
540540
- Creates a new PR for each branch that doesn't have one (base set to the first non-merged ancestor branch)
541541
- After creating PRs, links them together as a **Stack** on GitHub (requires the repository to have stacks enabled)
542+
- If stacks are not available (exit code 9), the repository does not have stacked PRs enabled. In interactive mode, `submit` offers to create regular (unstacked) PRs instead. In non-interactive mode, it exits with code 9.
542543
- Syncs PR metadata for branches that already have PRs
543544

544545
**PR title auto-generation (`--auto`):**
@@ -783,6 +784,7 @@ gh stack unstack feature-auth
783784
| 6 | Disambiguation required | A branch belongs to multiple stacks. Run `gh stack checkout <specific-branch>` to switch to a non-shared branch first |
784785
| 7 | Rebase already in progress | Run `gh stack rebase --continue` (after resolving conflicts) or `gh stack rebase --abort` to start over |
785786
| 8 | Stack is locked | Another `gh stack` process is writing the stack file. Wait and retry — the lock times out after 5 seconds |
787+
| 9 | Stacked PRs unavailable | The repository does not have stacked PRs enabled. `submit` will offer to create regular (unstacked) PRs in interactive mode |
786788

787789
## Known limitations
788790

0 commit comments

Comments
 (0)