Adopt existing remote stacks on submit

cmd/submit.go

@@ -563,9 +563,93 @@ func syncStack(cfg *config.Config, client github.ClientOps, s *stack.Stack) {
 
 	if s.ID != "" {
 		updateStack(cfg, client, s, prNumbers)
-	} else {
-		createNewStack(cfg, client, s, prNumbers)
+		return
+	}
+
+	// No locally tracked stack ID. The stack may already exist on GitHub
+	// (created from the web UI or another clone) without being recorded
+	// locally. Adopt it instead of blindly creating a new one, which the API
+	// rejects because the PRs are already part of a stack.
+	if adoptRemoteStack(cfg, client, s, prNumbers) {
+		return
 	}
+
+	createNewStack(cfg, client, s, prNumbers)
+}
+
+// adoptRemoteStack reconciles a locally untracked stack (s.ID == "") with the
+// stacks that already exist on GitHub. The PRs in s may already belong to a
+// remote stack created from the web UI or another clone; in that case we must
+// adopt that stack rather than POST a new one (which the API rejects because
+// the PRs are already stacked).
+//
+// It returns true when it has fully handled the sync — either by adopting and
+// updating the existing stack, or by intentionally refusing to modify a
+// divergent remote stack — and false when no matching remote stack exists and
+// the caller should create a new one.
+func adoptRemoteStack(cfg *config.Config, client github.ClientOps, s *stack.Stack, prNumbers []int) bool {
+	stacks, err := client.ListStacks()
+	if err != nil {
+		// Couldn't inspect remote state — fall back to the create path, which
+		// reports its own errors (handleCreate422 covers "already stacked").
+		return false
+	}
+
+	matched, err := findMatchingStack(stacks, prNumbers)
+	if err != nil {
+		// Our PRs are spread across more than one remote stack. A PR can only
+		// belong to one stack, so this is a genuine divergence we can't resolve
+		// automatically.
+		cfg.Warningf("Your PRs belong to multiple stacks on GitHub — reconcile them before submitting")
+		cfg.Printf("  Run `%s` to import a stack, or unstack the PRs from the web",
+			cfg.ColorCyan("gh stack checkout <pr>"))
+		return true
+	}
+
+	if matched == nil {
+		// No existing stack contains any of our PRs — create a new one.
+		return false
+	}
+
+	// A remote stack already contains some of our PRs. Refuse to silently drop
+	// any PRs it holds that we aren't tracking locally; let the user reconcile.
+	if dropped := prsMissingFrom(matched.PullRequests, prNumbers); len(dropped) > 0 {
+		cfg.Warningf("A stack on GitHub already contains %s, which %s not in your local stack",
+			formatPRList(dropped), plural(len(dropped), "is", "are"))
+		cfg.Printf("  Run `%s` to import the full stack, then `%s`",
+			cfg.ColorCyan("gh stack checkout <pr>"), cfg.ColorCyan("gh stack submit"))
+		return true
+	}
+
+	// Every PR in the remote stack is tracked locally (and we may have added
+	// more on top). Adopt the remote stack ID — recording it locally — and
+	// update the stack with our full, ordered PR list to append any new PRs.
+	s.ID = strconv.Itoa(matched.ID)
+
+	if slicesEqual(matched.PullRequests, prNumbers) {
+		cfg.Successf("Linked to the existing stack on GitHub (%d PRs, already up to date)", len(prNumbers))
+		return true
+	}
+
+	cfg.Infof("Found the stack on GitHub — updating it to match your local stack")
+	updateStack(cfg, client, s, prNumbers)
+	return true
+}
+
+// prsMissingFrom returns the numbers in remote that do not appear in local,
+// preserving remote order.
+func prsMissingFrom(remote, local []int) []int {
+	localSet := make(map[int]bool, len(local))
+	for _, n := range local {
+		localSet[n] = true
+	}
+	var missing []int
+	for _, n := range remote {
+		if !localSet[n] {
+			missing = append(missing, n)
+		}
+	}
+	return missing
 }
 
 // updateStack calls the PUT endpoint to sync the full PR list for an existing stack.
@@ -626,7 +710,7 @@ func createNewStack(cfg *config.Config, client github.ClientOps, s *stack.Stack,
 func handleCreate422(cfg *config.Config, httpErr *api.HTTPError, prNumbers []int) {
 	msg := httpErr.Message
 
-	if strings.Contains(msg, "already stacked") {
+	if isAlreadyStackedError(msg) {
 		// Check if the error lists exactly the same PRs we're trying to
 		// stack. If so, they're already in a stack together — nothing to do.
 		// If only a subset matches, the PRs are in a different stack.
@@ -635,8 +719,8 @@ func handleCreate422(cfg *config.Config, httpErr *api.HTTPError, prNumbers []int
 			return
 		}
 		cfg.Warningf("One or more PRs are already part of a different stack on GitHub")
-		cfg.Printf("  To fix this, unstack the PRs from the web, then `%s`",
-			cfg.ColorCyan("gh stack submit"))
+		cfg.Printf("  Run `%s` to import the existing stack, or unstack the PRs from the web",
+			cfg.ColorCyan("gh stack checkout <pr>"))
 		return
 	}
 
@@ -661,3 +745,13 @@ func allPRsInMessage(msg string, prNumbers []int) bool {
 	}
 	return true
 }
+
+// isAlreadyStackedError reports whether a create-stack 422 message indicates
+// the PRs already belong to a stack. The server has used more than one phrasing
+// ("Pull requests #1, #2 are already stacked", "Pull requests are already part
+// of a stack"), so match on the stable substrings rather than an exact string.
+func isAlreadyStackedError(msg string) bool {
+	m := strings.ToLower(msg)
+	return strings.Contains(m, "already stacked") ||
+		strings.Contains(m, "already part of a stack")
+}

cmd/submit_test.go

@@ -643,6 +643,243 @@ func TestSyncStack_AlreadyStacked_DifferentStack(t *testing.T) {
 	assert.NotContains(t, output, "up to date")
 }
 
+func TestSyncStack_AdoptsExistingRemoteStack_ExactMatch(t *testing.T) {
+	// The stack exists on GitHub but isn't recorded locally (s.ID == "").
+	// All local PRs match the remote stack exactly — adopt the ID without
+	// creating or updating anything.
+	s := &stack.Stack{
+		Trunk: stack.BranchRef{Branch: "main"},
+		Branches: []stack.BranchRef{
+			{Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 10}},
+			{Branch: "b2", PullRequest: &stack.PullRequestRef{Number: 11}},
+		},
+	}
+
+	var createCalled, updateCalled bool
+	mock := &github.MockClient{
+		ListStacksFn: func() ([]github.RemoteStack, error) {
+			return []github.RemoteStack{{ID: 77, PullRequests: []int{10, 11}}}, nil
+		},
+		CreateStackFn: func([]int) (int, error) {
+			createCalled = true
+			return 0, nil
+		},
+		UpdateStackFn: func(string, []int) error {
+			updateCalled = true
+			return nil
+		},
+	}
+
+	cfg, _, errR := config.NewTestConfig()
+	syncStack(cfg, mock, s)
+
+	cfg.Err.Close()
+	errOut, _ := io.ReadAll(errR)
+	output := string(errOut)
+
+	assert.False(t, createCalled, "should not create when the stack already exists on GitHub")
+	assert.False(t, updateCalled, "should not update when local matches remote exactly")
+	assert.Equal(t, "77", s.ID, "should adopt the remote stack ID into local tracking")
+	assert.Contains(t, output, "Linked to the existing stack on GitHub")
+	assert.Contains(t, output, "up to date")
+}
+
+func TestSyncStack_AdoptsExistingRemoteStack_AddsNewPR(t *testing.T) {
+	// Two of our three PRs already form a remote stack; the third was added
+	// locally on top. Adopt the remote ID and update the stack to include the
+	// new PR at the top.
+	s := &stack.Stack{
+		Trunk: stack.BranchRef{Branch: "main"},
+		Branches: []stack.BranchRef{
+			{Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 10}},
+			{Branch: "b2", PullRequest: &stack.PullRequestRef{Number: 11}},
+			{Branch: "b3", PullRequest: &stack.PullRequestRef{Number: 12}},
+		},
+	}
+
+	var createCalled bool
+	var gotStackID string
+	var gotNumbers []int
+	mock := &github.MockClient{
+		ListStacksFn: func() ([]github.RemoteStack, error) {
+			return []github.RemoteStack{{ID: 77, PullRequests: []int{10, 11}}}, nil
+		},
+		CreateStackFn: func([]int) (int, error) {
+			createCalled = true
+			return 0, nil
+		},
+		UpdateStackFn: func(stackID string, prNumbers []int) error {
+			gotStackID = stackID
+			gotNumbers = prNumbers
+			return nil
+		},
+	}
+
+	cfg, _, errR := config.NewTestConfig()
+	syncStack(cfg, mock, s)
+
+	cfg.Err.Close()
+	errOut, _ := io.ReadAll(errR)
+	output := string(errOut)
+
+	assert.False(t, createCalled, "should adopt and update, not create")
+	assert.Equal(t, "77", s.ID, "should adopt the remote stack ID")
+	assert.Equal(t, "77", gotStackID, "should update the adopted stack")
+	assert.Equal(t, []int{10, 11, 12}, gotNumbers, "should send the full local PR list")
+	assert.Contains(t, output, "Stack updated on GitHub with 3 PRs")
+}
+
+func TestSyncStack_RemoteStackHasExtraPRs_Refuses(t *testing.T) {
+	// The remote stack contains a PR we aren't tracking locally. Syncing to
+	// match local would drop it, so refuse and warn instead.
+	s := &stack.Stack{
+		Trunk: stack.BranchRef{Branch: "main"},
+		Branches: []stack.BranchRef{
+			{Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 10}},
+			{Branch: "b2", PullRequest: &stack.PullRequestRef{Number: 11}},
+		},
+	}
+
+	var createCalled, updateCalled bool
+	mock := &github.MockClient{
+		ListStacksFn: func() ([]github.RemoteStack, error) {
+			return []github.RemoteStack{{ID: 77, PullRequests: []int{10, 11, 12}}}, nil
+		},
+		CreateStackFn: func([]int) (int, error) {
+			createCalled = true
+			return 0, nil
+		},
+		UpdateStackFn: func(string, []int) error {
+			updateCalled = true
+			return nil
+		},
+	}
+
+	cfg, _, errR := config.NewTestConfig()
+	syncStack(cfg, mock, s)
+
+	cfg.Err.Close()
+	errOut, _ := io.ReadAll(errR)
+	output := string(errOut)
+
+	assert.False(t, createCalled, "should not create over an existing stack")
+	assert.False(t, updateCalled, "should not drop remote-only PRs")
+	assert.Equal(t, "", s.ID, "should not adopt a divergent remote stack")
+	assert.Contains(t, output, "#12")
+	assert.Contains(t, output, "not in your local stack")
+}
+
+func TestSyncStack_PRsSpanMultipleRemoteStacks_Warns(t *testing.T) {
+	// Our PRs are split across two remote stacks — an unresolvable divergence.
+	s := &stack.Stack{
+		Trunk: stack.BranchRef{Branch: "main"},
+		Branches: []stack.BranchRef{
+			{Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 10}},
+			{Branch: "b2", PullRequest: &stack.PullRequestRef{Number: 11}},
+		},
+	}
+
+	var createCalled, updateCalled bool
+	mock := &github.MockClient{
+		ListStacksFn: func() ([]github.RemoteStack, error) {
+			return []github.RemoteStack{
+				{ID: 1, PullRequests: []int{10}},
+				{ID: 2, PullRequests: []int{11}},
+			}, nil
+		},
+		CreateStackFn: func([]int) (int, error) {
+			createCalled = true
+			return 0, nil
+		},
+		UpdateStackFn: func(string, []int) error {
+			updateCalled = true
+			return nil
+		},
+	}
+
+	cfg, _, errR := config.NewTestConfig()
+	syncStack(cfg, mock, s)
+
+	cfg.Err.Close()
+	errOut, _ := io.ReadAll(errR)
+	output := string(errOut)
+
+	assert.False(t, createCalled, "should not create when PRs span multiple stacks")
+	assert.False(t, updateCalled, "should not update when PRs span multiple stacks")
+	assert.Equal(t, "", s.ID)
+	assert.Contains(t, output, "multiple stacks")
+}
+
+func TestSyncStack_ListStacksError_FallsThroughToCreate(t *testing.T) {
+	// If we can't inspect remote stacks, fall back to the create path rather
+	// than blocking the submit.
+	s := &stack.Stack{
+		Trunk: stack.BranchRef{Branch: "main"},
+		Branches: []stack.BranchRef{
+			{Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 10}},
+			{Branch: "b2", PullRequest: &stack.PullRequestRef{Number: 11}},
+		},
+	}
+
+	var createCalled bool
+	mock := &github.MockClient{
+		ListStacksFn: func() ([]github.RemoteStack, error) {
+			return nil, fmt.Errorf("network down")
+		},
+		CreateStackFn: func([]int) (int, error) {
+			createCalled = true
+			return 88, nil
+		},
+	}
+
+	cfg, _, errR := config.NewTestConfig()
+	syncStack(cfg, mock, s)
+
+	cfg.Err.Close()
+	errOut, _ := io.ReadAll(errR)
+	output := string(errOut)
+
+	assert.True(t, createCalled, "should fall through to CreateStack when ListStacks fails")
+	assert.Equal(t, "88", s.ID)
+	assert.Contains(t, output, "Stack created on GitHub with 2 PRs")
+}
+
+func TestSyncStack_AlreadyPartOfAStack_FallbackPhrasing(t *testing.T) {
+	// Fallback path: ListStacks returns no match (so adoption is skipped), but
+	// the create endpoint still rejects with the server's "already part of a
+	// stack" phrasing (no PR numbers). The message must be actionable rather
+	// than the raw "Could not create stack".
+	s := &stack.Stack{
+		Trunk: stack.BranchRef{Branch: "main"},
+		Branches: []stack.BranchRef{
+			{Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 10}},
+			{Branch: "b2", PullRequest: &stack.PullRequestRef{Number: 11}},
+		},
+	}
+
+	mock := &github.MockClient{
+		ListStacksFn: func() ([]github.RemoteStack, error) { return nil, nil },
+		CreateStackFn: func([]int) (int, error) {
+			return 0, &api.HTTPError{
+				StatusCode: 422,
+				Message:    "Pull requests are already part of a stack",
+				RequestURL: &url.URL{Path: "/repos/o/r/cli_internal/pulls/stacks"},
+			}
+		},
+	}
+
+	cfg, _, errR := config.NewTestConfig()
+	syncStack(cfg, mock, s)
+
+	cfg.Err.Close()
+	errOut, _ := io.ReadAll(errR)
+	output := string(errOut)
+
+	assert.Contains(t, output, "already part of a")
+	assert.Contains(t, output, "gh stack checkout")
+	assert.NotContains(t, output, "Could not create stack")
+}
+
 func TestSyncStack_InvalidChain_422(t *testing.T) {
 	s := &stack.Stack{
 		Trunk: stack.BranchRef{Branch: "main"},