Skip to content

Commit cbab0a1

Browse files
skarimskarim
committed
Cut over stack operations to the public Stacks REST API
Replace the private cli_internal stack endpoints with the new public Stacks REST API (/repos/{owner}/{repo}/stacks): - ListStacks / FindStackForPR (?pull_request= filter) / GetStack for reads - CreateStack, which now returns the created stack including its number - AddToStack for delta-only appends (there is no full-replace endpoint) - Unstack for server-driven removal (204 dissolved / 200 partial / 422) Migrate all callers (checkout, submit, link, sync, unstack, utils) and drop the client-side unstack eligibility pre-check — the server now decides which PRs can be unstacked. checkout discovers stacks via the pull_request filter; submit/link express updates as append-only deltas; unstack adopts partial-unstack semantics, keeping local tracking when PRs remain stacked on GitHub. RemoteStack now carries the stack number, and stack updates resolve a stack's number from its internal id for stack files that predate the Number field. Copilot-Session: 03673c26-a245-42da-93ed-dfcebc92a740
1 parent ae5ab09 commit cbab0a1

13 files changed

Lines changed: 924 additions & 655 deletions

‎cmd/checkout.go‎

Lines changed: 23 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -188,8 +188,9 @@ func checkoutRemoteStack(cfg *config.Config, sf *stack.StackFile, gitDir string,
188188
return nil, "", ErrAPIFailure
189189
}
190190

191-
// Step 1: List stacks and find one containing the target PR
192-
remoteStack, err := findRemoteStackForPR(client, prNumber)
191+
// Step 1: Find the stack containing the target PR via the list endpoint's
192+
// server-side pull_request filter.
193+
remoteStack, err := client.FindStackForPR(prNumber)
193194
if err != nil {
194195
var httpErr *api.HTTPError
195196
if errors.As(err, &httpErr) && httpErr.StatusCode == 404 {
@@ -205,7 +206,7 @@ func checkoutRemoteStack(cfg *config.Config, sf *stack.StackFile, gitDir string,
205206
}
206207

207208
// Step 2: Fetch PR details for every PR in the remote stack
208-
prs, err := fetchStackPRDetails(client, remoteStack.PullRequests)
209+
prs, err := fetchStackPRDetails(client, remoteStack.PRNumbers())
209210
if err != nil {
210211
cfg.Errorf("failed to fetch PR details: %v", err)
211212
return nil, "", ErrAPIFailure
@@ -245,11 +246,12 @@ func checkoutRemoteStack(cfg *config.Config, sf *stack.StackFile, gitDir string,
245246
syncRemotePRState(localStack, prs)
246247

247248
// Case A: branch is in a local stack — check composition
248-
if stackCompositionMatches(localStack, remoteStack.PullRequests) {
249+
if stackCompositionMatches(localStack, remoteStack.PRNumbers()) {
249250
// Composition matches — checkout
250251
if localStack.ID == "" {
251252
localStack.ID = remoteStackID
252253
}
254+
localStack.Number = remoteStack.Number
253255
if err := stack.Save(gitDir, sf); err != nil {
254256
return nil, "", handleSaveError(cfg, err)
255257
}
@@ -274,7 +276,7 @@ func checkoutRemoteStack(cfg *config.Config, sf *stack.StackFile, gitDir string,
274276
return nil, "", ErrSilent
275277
}
276278

277-
s, err := importRemoteStack(cfg, sf, gitDir, remote, trunk, prs, remoteStackID)
279+
s, err := importRemoteStack(cfg, sf, gitDir, remote, trunk, prs, remoteStackID, remoteStack.Number)
278280
if err != nil {
279281
return nil, "", err
280282
}
@@ -286,23 +288,6 @@ func checkoutRemoteStack(cfg *config.Config, sf *stack.StackFile, gitDir string,
286288
return s, targetBranch, nil
287289
}
288290

289-
// findRemoteStackForPR queries the list stacks API and returns the stack
290-
// containing the given PR number, or nil if no stack contains it.
291-
func findRemoteStackForPR(client github.ClientOps, prNumber int) (*github.RemoteStack, error) {
292-
stacks, err := client.ListStacks()
293-
if err != nil {
294-
return nil, err
295-
}
296-
for i := range stacks {
297-
for _, n := range stacks[i].PullRequests {
298-
if n == prNumber {
299-
return &stacks[i], nil
300-
}
301-
}
302-
}
303-
return nil, nil
304-
}
305-
306291
// fetchStackPRDetails fetches PR details for each number in the stack.
307292
// Returns PRs in the same order as the input numbers.
308293
func fetchStackPRDetails(client github.ClientOps, prNumbers []int) ([]*github.PullRequest, error) {
@@ -418,7 +403,7 @@ func handleCompositionConflict(
418403
return nil, ErrSilent
419404
}
420405

421-
s, importErr := importRemoteStack(cfg, sf, gitDir, remote, trunk, prs, remoteStackID)
406+
s, importErr := importRemoteStack(cfg, sf, gitDir, remote, trunk, prs, remoteStackID, remoteStack.Number)
422407
if importErr != nil {
423408
return nil, importErr
424409
}
@@ -429,19 +414,26 @@ func handleCompositionConflict(
429414
return s, nil
430415

431416
case 1:
432-
// Delete remote stack, keep local
433-
if err := client.DeleteStack(remoteStackID); err != nil {
417+
// Unstack the remote stack, keep local
418+
_, dissolved, err := client.Unstack(remoteStack.Number)
419+
if err != nil {
434420
var httpErr *api.HTTPError
435421
if errors.As(err, &httpErr) && httpErr.StatusCode == 404 {
436-
cfg.Warningf("Remote stack already deleted")
422+
cfg.Warningf("Remote stack already removed")
423+
} else if errors.As(err, &httpErr) && httpErr.StatusCode == 422 {
424+
cfg.Errorf("Cannot unstack remote stack: %s", httpErr.Message)
425+
return nil, ErrAPIFailure
437426
} else {
438-
cfg.Errorf("failed to delete remote stack: %v", err)
427+
cfg.Errorf("failed to unstack remote stack: %v", err)
439428
return nil, ErrAPIFailure
440429
}
430+
} else if dissolved {
431+
cfg.Successf("Remote stack removed")
441432
} else {
442-
cfg.Successf("Remote stack deleted")
433+
cfg.Warningf("Some pull requests could not be unstacked and remain on GitHub")
443434
}
444435
localStack.ID = ""
436+
localStack.Number = 0
445437
if err := stack.Save(gitDir, sf); err != nil {
446438
return nil, handleSaveError(cfg, err)
447439
}
@@ -475,6 +467,7 @@ func importRemoteStack(
475467
trunk string,
476468
prs []*github.PullRequest,
477469
remoteStackID string,
470+
remoteStackNumber int,
478471
) (*stack.Stack, error) {
479472
// Fetch latest refs from remote
480473
if err := git.Fetch(remote); err != nil {
@@ -512,7 +505,8 @@ func importRemoteStack(
512505

513506
trunkSHA, _ := git.RevParse(trunk)
514507
newStack := stack.Stack{
515-
ID: remoteStackID,
508+
ID: remoteStackID,
509+
Number: remoteStackNumber,
516510
Trunk: stack.BranchRef{
517511
Branch: trunk,
518512
Head: trunkSHA,

‎cmd/checkout_test.go‎

Lines changed: 32 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ func TestCheckout_NumericTarget_StacksNotAvailable(t *testing.T) {
160160
cfg, outR, errR := config.NewTestConfig()
161161
setTestTokenForHost(cfg, "gho_test_oauth_token")
162162
cfg.GitHubClientOverride = &github.MockClient{
163-
ListStacksFn: func() ([]github.RemoteStack, error) {
163+
FindStackForPRFn: func(int) (*github.RemoteStack, error) {
164164
return nil, &api.HTTPError{StatusCode: 404, Message: "Not Found"}
165165
},
166166
}
@@ -184,10 +184,8 @@ func TestCheckout_NumericTarget_PRNotInStack(t *testing.T) {
184184

185185
cfg, outR, errR := config.NewTestConfig()
186186
cfg.GitHubClientOverride = &github.MockClient{
187-
ListStacksFn: func() ([]github.RemoteStack, error) {
188-
return []github.RemoteStack{
189-
{ID: 1, PullRequests: []int{10, 11}},
190-
}, nil
187+
FindStackForPRFn: func(int) (*github.RemoteStack, error) {
188+
return nil, nil // PR 99 is not part of any stack
191189
},
192190
}
193191

@@ -243,10 +241,8 @@ func TestCheckout_NumericTarget_NewStack(t *testing.T) {
243241

244242
cfg, outR, errR := config.NewTestConfig()
245243
cfg.GitHubClientOverride = &github.MockClient{
246-
ListStacksFn: func() ([]github.RemoteStack, error) {
247-
return []github.RemoteStack{
248-
{ID: 42, PullRequests: []int{10, 11, 12}},
249-
}, nil
244+
FindStackForPRFn: func(int) (*github.RemoteStack, error) {
245+
return &github.RemoteStack{ID: 42, Number: 42, PullRequests: []int{10, 11, 12}}, nil
250246
},
251247
FindPRByNumberFn: func(number int) (*github.PullRequest, error) {
252248
prs := map[int]*github.PullRequest{
@@ -331,10 +327,8 @@ func TestCheckout_NumericTarget_BranchExistsNoStack(t *testing.T) {
331327

332328
cfg, outR, errR := config.NewTestConfig()
333329
cfg.GitHubClientOverride = &github.MockClient{
334-
ListStacksFn: func() ([]github.RemoteStack, error) {
335-
return []github.RemoteStack{
336-
{ID: 99, PullRequests: []int{10, 11}},
337-
}, nil
330+
FindStackForPRFn: func(int) (*github.RemoteStack, error) {
331+
return &github.RemoteStack{ID: 99, Number: 99, PullRequests: []int{10, 11}}, nil
338332
},
339333
FindPRByNumberFn: func(number int) (*github.PullRequest, error) {
340334
prs := map[int]*github.PullRequest{
@@ -445,11 +439,9 @@ func TestCheckout_NumericTarget_LocalMiss_RemoteMatch(t *testing.T) {
445439
apiCalled := false
446440
cfg, outR, errR := config.NewTestConfig()
447441
cfg.GitHubClientOverride = &github.MockClient{
448-
ListStacksFn: func() ([]github.RemoteStack, error) {
442+
FindStackForPRFn: func(int) (*github.RemoteStack, error) {
449443
apiCalled = true
450-
return []github.RemoteStack{
451-
{ID: 99, PullRequests: []int{10, 11}},
452-
}, nil
444+
return &github.RemoteStack{ID: 99, Number: 99, PullRequests: []int{10, 11}}, nil
453445
},
454446
FindPRByNumberFn: func(number int) (*github.PullRequest, error) {
455447
prs := map[int]*github.PullRequest{
@@ -464,7 +456,7 @@ func TestCheckout_NumericTarget_LocalMiss_RemoteMatch(t *testing.T) {
464456
_ = collectOutput(cfg, outR, errR)
465457

466458
require.NoError(t, err)
467-
assert.True(t, apiCalled, "should have called ListStacks API when local miss")
459+
assert.True(t, apiCalled, "should have queried the remote stack API when local miss")
468460
assert.Equal(t, "feat-2", checkedOut)
469461
}
470462

@@ -493,8 +485,8 @@ func TestCheckout_NumericTarget_FallbackToBranchName(t *testing.T) {
493485

494486
cfg, outR, errR := config.NewTestConfig()
495487
cfg.GitHubClientOverride = &github.MockClient{
496-
ListStacksFn: func() ([]github.RemoteStack, error) {
497-
return []github.RemoteStack{}, nil // no remote stacks
488+
FindStackForPRFn: func(int) (*github.RemoteStack, error) {
489+
return nil, nil // no remote stack contains this PR
498490
},
499491
}
500492

@@ -527,11 +519,9 @@ func TestCheckout_NumericTarget_CompositionMismatch_NonInteractive(t *testing.T)
527519

528520
cfg, outR, errR := config.NewTestConfig()
529521
cfg.GitHubClientOverride = &github.MockClient{
530-
ListStacksFn: func() ([]github.RemoteStack, error) {
522+
FindStackForPRFn: func(int) (*github.RemoteStack, error) {
531523
// Remote stack has PRs 10, 11, 12 (extra PR added)
532-
return []github.RemoteStack{
533-
{ID: 42, PullRequests: []int{10, 11, 12}},
534-
}, nil
524+
return &github.RemoteStack{ID: 42, Number: 42, PullRequests: []int{10, 11, 12}}, nil
535525
},
536526
FindPRByNumberFn: func(number int) (*github.PullRequest, error) {
537527
prs := map[int]*github.PullRequest{
@@ -590,10 +580,8 @@ func TestCheckout_NumericTarget_ClosedMergedPR(t *testing.T) {
590580

591581
cfg, outR, errR := config.NewTestConfig()
592582
cfg.GitHubClientOverride = &github.MockClient{
593-
ListStacksFn: func() ([]github.RemoteStack, error) {
594-
return []github.RemoteStack{
595-
{ID: 50, PullRequests: []int{10, 11}},
596-
}, nil
583+
FindStackForPRFn: func(int) (*github.RemoteStack, error) {
584+
return &github.RemoteStack{ID: 50, Number: 50, PullRequests: []int{10, 11}}, nil
597585
},
598586
FindPRByNumberFn: func(number int) (*github.PullRequest, error) {
599587
prs := map[int]*github.PullRequest{
@@ -662,10 +650,8 @@ func TestCheckout_NumericTarget_MergedBranchDeletedFromRemote(t *testing.T) {
662650

663651
cfg, outR, errR := config.NewTestConfig()
664652
cfg.GitHubClientOverride = &github.MockClient{
665-
ListStacksFn: func() ([]github.RemoteStack, error) {
666-
return []github.RemoteStack{
667-
{ID: 60, PullRequests: []int{10, 11}},
668-
}, nil
653+
FindStackForPRFn: func(int) (*github.RemoteStack, error) {
654+
return &github.RemoteStack{ID: 60, Number: 60, PullRequests: []int{10, 11}}, nil
669655
},
670656
FindPRByNumberFn: func(number int) (*github.PullRequest, error) {
671657
prs := map[int]*github.PullRequest{
@@ -698,10 +684,8 @@ func TestCheckout_NumericTarget_AllPRsMerged(t *testing.T) {
698684

699685
cfg, outR, errR := config.NewTestConfig()
700686
cfg.GitHubClientOverride = &github.MockClient{
701-
ListStacksFn: func() ([]github.RemoteStack, error) {
702-
return []github.RemoteStack{
703-
{ID: 70, PullRequests: []int{10, 11}},
704-
}, nil
687+
FindStackForPRFn: func(int) (*github.RemoteStack, error) {
688+
return &github.RemoteStack{ID: 70, Number: 70, PullRequests: []int{10, 11}}, nil
705689
},
706690
FindPRByNumberFn: func(number int) (*github.PullRequest, error) {
707691
prs := map[int]*github.PullRequest{
@@ -732,7 +716,7 @@ func TestCheckout_NumericTarget_APIError(t *testing.T) {
732716

733717
cfg, outR, errR := config.NewTestConfig()
734718
cfg.GitHubClientOverride = &github.MockClient{
735-
ListStacksFn: func() ([]github.RemoteStack, error) {
719+
FindStackForPRFn: func(int) (*github.RemoteStack, error) {
736720
return nil, fmt.Errorf("network error")
737721
},
738722
}
@@ -796,8 +780,8 @@ func TestCheckout_NumericTarget_EmptyStacks(t *testing.T) {
796780

797781
cfg, outR, errR := config.NewTestConfig()
798782
cfg.GitHubClientOverride = &github.MockClient{
799-
ListStacksFn: func() ([]github.RemoteStack, error) {
800-
return []github.RemoteStack{}, nil // no stacks at all
783+
FindStackForPRFn: func(int) (*github.RemoteStack, error) {
784+
return nil, nil // no stacks at all
801785
},
802786
}
803787

@@ -903,34 +887,6 @@ func TestStackCompositionMatches(t *testing.T) {
903887
}
904888
}
905889

906-
func TestFindRemoteStackForPR(t *testing.T) {
907-
mock := &github.MockClient{
908-
ListStacksFn: func() ([]github.RemoteStack, error) {
909-
return []github.RemoteStack{
910-
{ID: 1, PullRequests: []int{10, 11}},
911-
{ID: 2, PullRequests: []int{20, 21, 22}},
912-
}, nil
913-
},
914-
}
915-
916-
// Found in first stack
917-
rs, err := findRemoteStackForPR(mock, 11)
918-
require.NoError(t, err)
919-
require.NotNil(t, rs)
920-
assert.Equal(t, 1, rs.ID)
921-
922-
// Found in second stack
923-
rs, err = findRemoteStackForPR(mock, 21)
924-
require.NoError(t, err)
925-
require.NotNil(t, rs)
926-
assert.Equal(t, 2, rs.ID)
927-
928-
// Not found
929-
rs, err = findRemoteStackForPR(mock, 99)
930-
require.NoError(t, err)
931-
assert.Nil(t, rs)
932-
}
933-
934890
func TestCheckout_ByPRURL_Local(t *testing.T) {
935891
// When a PR URL resolves to a locally tracked stack, no API call needed
936892
gitDir := t.TempDir()
@@ -973,14 +929,14 @@ func TestCheckout_ByPRURL_Remote(t *testing.T) {
973929
}
974930

975931
restore := git.SetOps(&git.MockOps{
976-
GitDirFn: func() (string, error) { return gitDir, nil },
977-
CurrentBranchFn: func() (string, error) { return "main", nil },
978-
BranchExistsFn: func(name string) bool { return name == "main" },
979-
FetchFn: func(string) error { return nil },
980-
CreateBranchFn: func(string, string) error { return nil },
932+
GitDirFn: func() (string, error) { return gitDir, nil },
933+
CurrentBranchFn: func() (string, error) { return "main", nil },
934+
BranchExistsFn: func(name string) bool { return name == "main" },
935+
FetchFn: func(string) error { return nil },
936+
CreateBranchFn: func(string, string) error { return nil },
981937
SetUpstreamTrackingFn: func(string, string) error { return nil },
982-
RevParseFn: func(string) (string, error) { return "abc123", nil },
983-
ResolveRemoteFn: func(string) (string, error) { return "origin", nil },
938+
RevParseFn: func(string) (string, error) { return "abc123", nil },
939+
ResolveRemoteFn: func(string) (string, error) { return "origin", nil },
984940
CheckoutBranchFn: func(name string) error {
985941
checkedOut = name
986942
return nil
@@ -993,10 +949,8 @@ func TestCheckout_ByPRURL_Remote(t *testing.T) {
993949

994950
cfg, outR, errR := config.NewTestConfig()
995951
cfg.GitHubClientOverride = &github.MockClient{
996-
ListStacksFn: func() ([]github.RemoteStack, error) {
997-
return []github.RemoteStack{
998-
{ID: 1, PullRequests: []int{10, 11}},
999-
}, nil
952+
FindStackForPRFn: func(int) (*github.RemoteStack, error) {
953+
return &github.RemoteStack{ID: 1, Number: 1, PullRequests: []int{10, 11}}, nil
1000954
},
1001955
FindPRByNumberFn: func(n int) (*github.PullRequest, error) {
1002956
if pr, ok := prDB[n]; ok {

0 commit comments

Comments
 (0)