Skip to content

Commit f308f8a

Browse files
skarimskarim
committed
unstack as a pure api wrapper if stack not checked out locally
1 parent d7e0313 commit f308f8a

3 files changed

Lines changed: 291 additions & 56 deletions

File tree

‎cmd/unstack.go‎

Lines changed: 122 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66

77
"github.com/cli/go-gh/v2/pkg/api"
88
"github.com/github/gh-stack/internal/config"
9+
"github.com/github/gh-stack/internal/github"
910
"github.com/github/gh-stack/internal/modify"
1011
"github.com/github/gh-stack/internal/stack"
1112
"github.com/spf13/cobra"
@@ -25,17 +26,24 @@ func UnstackCmd(cfg *config.Config) *cobra.Command {
2526
Short: "Remove a stack locally and on GitHub",
2627
Long: `Remove a stack from local tracking and unstack it on GitHub.
2728
28-
With no argument, the current active stack is used. Provide a stack number (the
29-
identifier shown in the github.com stack UI) to unstack a specific locally
30-
tracked stack instead. Use --local to only remove local tracking.
29+
With no argument, the active stack (the one containing the currently checked out
30+
branch) is unstacked on GitHub and removed from local tracking.
31+
32+
Provide a stack number (the identifier shown in the GitHub stack UI) to
33+
unstack a specific stack on GitHub. This works from anywhere in the repository,
34+
whether or not the stack is checked out locally — the number is unstacked
35+
directly through the GitHub API. If the stack is also available locally, its
36+
local tracking is removed as well.
37+
38+
Use --local to only remove local tracking without touching remote (GitHub).
3139
3240
GitHub decides which pull requests can be unstacked: PRs that are queued for
3341
merge or have auto-merge enabled are left stacked. When some pull requests
34-
remain stacked, local tracking is kept.`,
42+
remain stacked, the stack is kept (and local tracking, if any, is unchanged).`,
3543
Example: ` # Unstack the current stack locally and on GitHub
3644
$ gh stack unstack
3745
38-
# Unstack a specific stack by its stack number
46+
# Unstack a specific stack by its number
3947
$ gh stack unstack 7
4048
4149
# Only remove local tracking (keep the stack on GitHub)
@@ -60,16 +68,39 @@ remain stacked, local tracking is kept.`,
6068
}
6169

6270
func runUnstack(cfg *config.Config, opts *unstackOptions) error {
63-
var result *loadStackResult
64-
var err error
71+
// A stack number targets a specific stack. It is unstacked directly on
72+
// GitHub by number (remote-first), so this works from anywhere in the
73+
// repository whether or not the stack is tracked locally.
6574
if opts.stackNumber > 0 {
66-
result, err = loadStackByNumber(cfg, opts.stackNumber)
67-
} else {
68-
result, err = loadStack(cfg, "")
75+
result, ok, err := lookupStackByNumber(cfg, opts.stackNumber)
76+
if err != nil {
77+
return ErrNotInStack
78+
}
79+
if !ok {
80+
// The stack number isn't tracked locally.
81+
if opts.local {
82+
// --local never contacts GitHub, and there is nothing to remove
83+
// locally, so there is nothing to do.
84+
cfg.Errorf("stack #%d is not tracked locally", opts.stackNumber)
85+
cfg.Printf("Omit %s to unstack it on GitHub", cfg.ColorCyan("--local"))
86+
return ErrNotInStack
87+
}
88+
return runRemoteUnstack(cfg, opts.stackNumber)
89+
}
90+
return unstackTrackedStack(cfg, opts, result)
6991
}
92+
93+
// No argument: operate on the active stack for the current branch.
94+
result, err := loadStack(cfg, "")
7095
if err != nil {
7196
return ErrNotInStack
7297
}
98+
return unstackTrackedStack(cfg, opts, result)
99+
}
100+
101+
// unstackTrackedStack unstacks a locally tracked stack: it removes the stack on
102+
// GitHub (unless --local) and then removes it from local tracking.
103+
func unstackTrackedStack(cfg *config.Config, opts *unstackOptions, result *loadStackResult) error {
73104
gitDir := result.GitDir
74105

75106
if err := modify.CheckStateGuard(gitDir); err != nil {
@@ -102,35 +133,16 @@ func runUnstack(cfg *config.Config, opts *unstackOptions) error {
102133

103134
if number == 0 {
104135
cfg.Warningf("Stack not found on GitHub — continuing with local unstack")
105-
} else if _, dissolved, err := client.Unstack(number); err != nil {
106-
var httpErr *api.HTTPError
107-
if errors.As(err, &httpErr) {
108-
switch httpErr.StatusCode {
109-
case 404:
110-
// Stack already gone on GitHub — treat as success.
111-
cfg.Warningf("Stack not found on GitHub — continuing with local unstack")
112-
case 422:
113-
// The server refused: every PR is queued for merge or has
114-
// auto-merge enabled, so nothing can be unstacked.
115-
cfg.Errorf("Unstacking not allowed: %s", httpErr.Message)
116-
return ErrInvalidArgs
117-
default:
118-
cfg.Errorf("Failed to unstack on GitHub (HTTP %d): %s", httpErr.StatusCode, httpErr.Message)
119-
return ErrAPIFailure
120-
}
121-
} else {
122-
cfg.Errorf("Failed to unstack on GitHub: %v", err)
123-
return ErrAPIFailure
124-
}
125-
} else if !dissolved {
126-
// Some PRs (queued for merge or with auto-merge enabled) remain
127-
// stacked on GitHub, so the stack still exists. Keep local
128-
// tracking so it continues to reflect the remote stack.
129-
cfg.Warningf("Some pull requests are queued for merge or have auto-merge enabled and remain stacked on GitHub")
130-
cfg.Printf("The stack was left in place — local tracking is unchanged")
131-
return nil
132136
} else {
133-
cfg.Successf("Stack removed on GitHub%s", stackLabel(number))
137+
keepLocal, err := unstackNumberOnGitHub(cfg, client, number, true)
138+
if err != nil {
139+
return err
140+
}
141+
if keepLocal {
142+
// Some PRs remain stacked, so the stack still exists on
143+
// GitHub. Keep local tracking so it continues to reflect it.
144+
return nil
145+
}
134146
}
135147
}
136148
}
@@ -151,3 +163,75 @@ func runUnstack(cfg *config.Config, opts *unstackOptions) error {
151163

152164
return nil
153165
}
166+
167+
// runRemoteUnstack unstacks a stack on GitHub purely by its number, without any
168+
// local tracking. This is the remote-first path that lets `gh stack unstack
169+
// <number>` run from anywhere in the repository (like `gh stack link`), whether
170+
// or not the stack is checked out locally.
171+
func runRemoteUnstack(cfg *config.Config, number int) error {
172+
client, err := cfg.GitHubClient()
173+
if err != nil {
174+
cfg.Errorf("failed to create GitHub client: %s", err)
175+
return ErrAPIFailure
176+
}
177+
if _, err := unstackNumberOnGitHub(cfg, client, number, false); err != nil {
178+
return err
179+
}
180+
return nil
181+
}
182+
183+
// unstackNumberOnGitHub calls the Unstack API for the given stack number and
184+
// reports the outcome. hasLocalTracking indicates whether the caller has a
185+
// locally tracked stack to reconcile, which changes how a 404 and a partial
186+
// unstack are handled: with local tracking a 404 is an idempotent success (the
187+
// caller finishes removing local state) and a partial unstack keeps local
188+
// tracking; without it a 404 is a hard error because the user targeted a stack
189+
// that does not exist on GitHub.
190+
//
191+
// It returns keepLocal=true when local tracking should be preserved because a
192+
// partial unstack left some PRs stacked. keepLocal is only meaningful when
193+
// hasLocalTracking is true.
194+
func unstackNumberOnGitHub(cfg *config.Config, client github.ClientOps, number int, hasLocalTracking bool) (keepLocal bool, err error) {
195+
_, dissolved, err := client.Unstack(number)
196+
if err != nil {
197+
var httpErr *api.HTTPError
198+
if errors.As(err, &httpErr) {
199+
switch httpErr.StatusCode {
200+
case 404:
201+
if hasLocalTracking {
202+
// Stack already gone on GitHub — treat as success and let
203+
// the caller finish removing local tracking.
204+
cfg.Warningf("Stack not found on GitHub — continuing with local unstack")
205+
return false, nil
206+
}
207+
// Remote-first: the targeted stack does not exist on GitHub.
208+
cfg.Errorf("stack #%d not found on GitHub", number)
209+
return false, ErrNotInStack
210+
case 422:
211+
// The server refused: every PR is queued for merge or has
212+
// auto-merge enabled, so nothing can be unstacked.
213+
cfg.Errorf("Unstacking not allowed: %s", httpErr.Message)
214+
return false, ErrInvalidArgs
215+
default:
216+
cfg.Errorf("Failed to unstack on GitHub (HTTP %d): %s", httpErr.StatusCode, httpErr.Message)
217+
return false, ErrAPIFailure
218+
}
219+
}
220+
cfg.Errorf("Failed to unstack on GitHub: %v", err)
221+
return false, ErrAPIFailure
222+
}
223+
224+
if !dissolved {
225+
// Some PRs (queued for merge or with auto-merge enabled) remain stacked
226+
// on GitHub, so the stack still exists.
227+
cfg.Warningf("Some pull requests are queued for merge or have auto-merge enabled and remain stacked on GitHub")
228+
if hasLocalTracking {
229+
cfg.Printf("The stack was left in place — local tracking is unchanged")
230+
return true, nil
231+
}
232+
return false, nil
233+
}
234+
235+
cfg.Successf("Stack removed on GitHub%s", stackLabel(number))
236+
return false, nil
237+
}

‎cmd/unstack_test.go‎

Lines changed: 151 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,155 @@ func TestUnstack_ByStackNumber(t *testing.T) {
449449
assert.Equal(t, []string{"b1", "b2"}, sf.Stacks[0].BranchNames())
450450
}
451451

452-
func TestUnstack_ByStackNumber_NotTrackedLocally(t *testing.T) {
452+
func TestUnstack_ByStackNumber_RemoteOnly_Dissolved(t *testing.T) {
453+
// A stack number that isn't tracked locally is unstacked directly on GitHub
454+
// (remote-first), leaving unrelated local tracking untouched.
455+
gitDir := t.TempDir()
456+
restore := git.SetOps(&git.MockOps{
457+
GitDirFn: func() (string, error) { return gitDir, nil },
458+
CurrentBranchFn: func() (string, error) { return "b1", nil },
459+
})
460+
defer restore()
461+
462+
writeStackFile(t, gitDir, stack.Stack{
463+
ID: "42",
464+
Number: 42,
465+
Trunk: stack.BranchRef{Branch: "main"},
466+
Branches: []stack.BranchRef{{Branch: "b1"}, {Branch: "b2"}},
467+
})
468+
469+
var unstackedNumber int
470+
cfg, outR, errR := config.NewTestConfig()
471+
cfg.GitHubClientOverride = &github.MockClient{
472+
UnstackFn: func(n int) (*github.RemoteStack, bool, error) {
473+
unstackedNumber = n
474+
return nil, true, nil // dissolved
475+
},
476+
}
477+
err := runUnstack(cfg, &unstackOptions{stackNumber: 999})
478+
output := collectOutput(cfg, outR, errR)
479+
480+
require.NoError(t, err)
481+
assert.Equal(t, 999, unstackedNumber, "should unstack the requested number on GitHub")
482+
assert.Contains(t, output, "Stack removed on GitHub")
483+
// No local tracking was touched.
484+
assert.NotContains(t, output, "Stack removed from local tracking")
485+
486+
sf, err := stack.Load(gitDir)
487+
require.NoError(t, err)
488+
require.Len(t, sf.Stacks, 1)
489+
assert.Equal(t, 42, sf.Stacks[0].Number, "the unrelated local stack is left intact")
490+
}
491+
492+
func TestUnstack_ByStackNumber_RemoteOnly_NotFound(t *testing.T) {
493+
// With no local tracking to reconcile, a 404 means the targeted stack does
494+
// not exist on GitHub — a hard error, not an idempotent success.
495+
gitDir := t.TempDir()
496+
restore := git.SetOps(&git.MockOps{
497+
GitDirFn: func() (string, error) { return gitDir, nil },
498+
CurrentBranchFn: func() (string, error) { return "b1", nil },
499+
})
500+
defer restore()
501+
502+
writeStackFile(t, gitDir, stack.Stack{
503+
ID: "42",
504+
Number: 42,
505+
Trunk: stack.BranchRef{Branch: "main"},
506+
Branches: []stack.BranchRef{{Branch: "b1"}, {Branch: "b2"}},
507+
})
508+
509+
cfg, outR, errR := config.NewTestConfig()
510+
cfg.GitHubClientOverride = &github.MockClient{
511+
UnstackFn: func(int) (*github.RemoteStack, bool, error) {
512+
return nil, false, &api.HTTPError{StatusCode: 404, Message: "Not Found"}
513+
},
514+
}
515+
err := runUnstack(cfg, &unstackOptions{stackNumber: 999})
516+
output := collectOutput(cfg, outR, errR)
517+
518+
assert.ErrorIs(t, err, ErrNotInStack)
519+
assert.Contains(t, output, "stack #999 not found on GitHub")
520+
assert.NotContains(t, output, "Stack removed")
521+
522+
sf, err := stack.Load(gitDir)
523+
require.NoError(t, err)
524+
require.Len(t, sf.Stacks, 1)
525+
}
526+
527+
func TestUnstack_ByStackNumber_RemoteOnly_Partial(t *testing.T) {
528+
// Some PRs (queued for merge / auto-merge) remain stacked. There is no local
529+
// tracking to keep, so the command reports the outcome and succeeds.
530+
gitDir := t.TempDir()
531+
restore := git.SetOps(&git.MockOps{
532+
GitDirFn: func() (string, error) { return gitDir, nil },
533+
CurrentBranchFn: func() (string, error) { return "b1", nil },
534+
})
535+
defer restore()
536+
537+
writeStackFile(t, gitDir, stack.Stack{
538+
ID: "42",
539+
Number: 42,
540+
Trunk: stack.BranchRef{Branch: "main"},
541+
Branches: []stack.BranchRef{{Branch: "b1"}, {Branch: "b2"}},
542+
})
543+
544+
cfg, outR, errR := config.NewTestConfig()
545+
cfg.GitHubClientOverride = &github.MockClient{
546+
UnstackFn: func(int) (*github.RemoteStack, bool, error) {
547+
return &github.RemoteStack{ID: 555, Number: 999, PullRequests: []int{102}}, false, nil
548+
},
549+
}
550+
err := runUnstack(cfg, &unstackOptions{stackNumber: 999})
551+
output := collectOutput(cfg, outR, errR)
552+
553+
require.NoError(t, err)
554+
assert.Contains(t, output, "remain stacked on GitHub")
555+
// No local tracking is involved, so no local-tracking messaging is shown.
556+
assert.NotContains(t, output, "local tracking is unchanged")
557+
assert.NotContains(t, output, "Stack removed from local tracking")
558+
559+
sf, err := stack.Load(gitDir)
560+
require.NoError(t, err)
561+
require.Len(t, sf.Stacks, 1)
562+
}
563+
564+
func TestUnstack_ByStackNumber_RemoteOnly_AllLocked(t *testing.T) {
565+
// Every PR is queued for merge or has auto-merge enabled; the server rejects
566+
// the unstack with a 422 and the command surfaces the error.
567+
gitDir := t.TempDir()
568+
restore := git.SetOps(&git.MockOps{
569+
GitDirFn: func() (string, error) { return gitDir, nil },
570+
CurrentBranchFn: func() (string, error) { return "b1", nil },
571+
})
572+
defer restore()
573+
574+
writeStackFile(t, gitDir, stack.Stack{
575+
ID: "42",
576+
Number: 42,
577+
Trunk: stack.BranchRef{Branch: "main"},
578+
Branches: []stack.BranchRef{{Branch: "b1"}, {Branch: "b2"}},
579+
})
580+
581+
cfg, outR, errR := config.NewTestConfig()
582+
cfg.GitHubClientOverride = &github.MockClient{
583+
UnstackFn: func(int) (*github.RemoteStack, bool, error) {
584+
return nil, false, &api.HTTPError{StatusCode: 422, Message: "all pull requests are queued for merge or have auto-merge enabled"}
585+
},
586+
}
587+
err := runUnstack(cfg, &unstackOptions{stackNumber: 999})
588+
output := collectOutput(cfg, outR, errR)
589+
590+
assert.ErrorIs(t, err, ErrInvalidArgs)
591+
assert.Contains(t, output, "Unstacking not allowed")
592+
593+
sf, err := stack.Load(gitDir)
594+
require.NoError(t, err)
595+
require.Len(t, sf.Stacks, 1)
596+
}
597+
598+
func TestUnstack_ByStackNumber_NotTracked_LocalFlag(t *testing.T) {
599+
// --local never contacts GitHub. Targeting a number that isn't tracked
600+
// locally with --local is an error: there is nothing to remove locally.
453601
gitDir := t.TempDir()
454602
restore := git.SetOps(&git.MockOps{
455603
GitDirFn: func() (string, error) { return gitDir, nil },
@@ -472,11 +620,11 @@ func TestUnstack_ByStackNumber_NotTrackedLocally(t *testing.T) {
472620
return nil, true, nil
473621
},
474622
}
475-
err := runUnstack(cfg, &unstackOptions{stackNumber: 999})
623+
err := runUnstack(cfg, &unstackOptions{stackNumber: 999, local: true})
476624
output := collectOutput(cfg, outR, errR)
477625

478626
assert.ErrorIs(t, err, ErrNotInStack)
479-
assert.False(t, unstackCalled, "should not unstack when the number isn't tracked locally")
627+
assert.False(t, unstackCalled, "--local must never contact GitHub")
480628
assert.Contains(t, output, "stack #999 is not tracked locally")
481629

482630
sf, err := stack.Load(gitDir)

0 commit comments

Comments
 (0)