Skip to content

Commit c3653a2

Browse files
committed
Create/update the remote stack on sync and fix false "Stack synced"
`gh stack sync` reported "Stack synced" even when it had not created or updated the stack object on GitHub. After running `gh stack init` to adopt existing branches and then opening PRs outside the CLI, `gh stack sync` detected the open PRs and printed "Stack synced" — but no stack had ever been created on the server. There were two distinct bugs: 1. Sync never reconciled the remote stack object. `runSync` called `syncStackPRs`, which only *reads* PR state and links PRs to local branches; it never called the create/update path. So the branches were rebased and pushed and the PRs were detected, but the stack on GitHub was never created. 2. The final message was unconditional. `runSync` always printed "Stack synced", which is supposed to mean "the stack object on GitHub now reflects the local stack" — something that can only be true when two or more open PRs exist and the remote stack was actually created/updated. Fix Reconcile the remote stack from sync, and make the closing message reflect what actually happened. * cmd/sync.go - Add a reconciliation step (5b) after PR-state sync: when the stack has two or more open PRs, link them into a stack on GitHub via the new `syncRemoteStack` helper. It inspects existing stacks first and: - short-circuits quietly when a remote stack already lists exactly these PRs (records the ID, prints "Stack already up to date on GitHub") so routine syncs don't issue a redundant, misleading update; - otherwise delegates to `syncStack` to create a new stack, adopt an untracked one, or update a partially-formed one. Sync never opens PRs — that remains `gh stack submit`'s job. - Replace the unconditional "Stack synced" with a result-driven message: "Stack synced" when the remote stack object was created/updated/in sync, otherwise "Branches synced" (fewer than two PRs, stacked PRs unavailable, a cross-stack divergence, or no GitHub client). - Update the command's long description to document the stack-object step and the two possible closing messages. * cmd/submit.go - Thread a `synced bool` return through the existing, tested stack helpers so sync can tell whether the remote stack object now matches local: `syncStack`, `createNewStack`, and `updateStack` now return `bool`; `adoptRemoteStack` returns `(handled, synced)`; and `handleCreate422` returns `bool` (true only when the PRs are already stacked together). Extract the shared `stackPRNumbers` helper. - This is additive: submit's single call site ignores the new return value, so submit's behavior, output, and tests are unchanged. Reusing these helpers (instead of duplicating the 404/422 handling in sync) keeps the create/adopt/update logic in one tested place. Tests * cmd/sync_test.go — six new cases covering the reconciliation matrix: - TestSync_CreatesRemoteStackWhenPRsExist: open PRs but no remote stack -> CreateStack is called and the new ID is persisted to the stack file; output contains "Stack created on GitHub" and "Stack synced". - TestSync_AdoptsExistingEqualRemoteStack: a matching remote stack -> no create/update, ID recorded, "Stack synced". - TestSync_UpdatesPartialRemoteStack: a subset stack -> UpdateStack with the full PR list, "Stack synced". - TestSync_FewerThanTwoPRs_BranchesSynced: one PR -> no stack API calls, "Branches synced", not "Stack synced". - TestSync_StacksUnavailable_BranchesSynced: 404 on create -> warns, "Branches synced". - TestSync_PRsSpanMultipleStacks_BranchesSynced: PRs across two stacks -> divergence warning, no create/update, "Branches synced". Docs Document the new stack-object step and the "Stack synced" vs "Branches synced" distinction in: - README.md - docs/src/content/docs/reference/cli.md - skills/gh-stack/SKILL.md - docs/src/content/docs/introduction/overview.md - docs/src/content/docs/guides/stacked-prs.md - docs/src/content/docs/guides/workflows.md
1 parent a9fa574 commit c3653a2

9 files changed

Lines changed: 401 additions & 48 deletions

File tree

README.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,8 @@ Performs a safe, non-interactive synchronization of the entire stack:
323323
3. **Cascade rebase** — rebases all stack branches onto their updated parents (only if trunk moved). If a conflict is detected, all branches are restored to their original state and you are advised to run `gh stack rebase` to resolve conflicts interactively
324324
4. **Push** — pushes all branches (uses `--force-with-lease` if a rebase occurred)
325325
5. **Sync PRs** — syncs PR state from GitHub and reports the status of each PR
326-
6. **Prune** — in interactive terminals, prompts to delete local branches for merged PRs. Use `--prune` to prune automatically
326+
6. **Sync the stack** — links the stack's open PRs into a stack on GitHub, creating the remote stack object if it doesn't exist yet or updating it if it's partially formed. Only happens when two or more PRs exist; sync never opens PRs (use `gh stack submit` for that)
327+
7. **Prune** — in interactive terminals, prompts to delete local branches for merged PRs. Use `--prune` to prune automatically
327328

328329
| Flag | Description |
329330
|------|-------------|

cmd/submit.go

Lines changed: 57 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -684,42 +684,50 @@ func clearPendingModifyState(cfg *config.Config, gitDir string) {
684684
cfg.Successf("Stack recreated on GitHub to match local state")
685685
}
686686

687-
// syncStack creates or updates a stack on GitHub from the active PRs.
688-
// If the stack already exists (s.ID is set), it calls the PUT endpoint with
689-
// the full list of PRs to keep the remote stack in sync. If no stack exists
690-
// yet, it calls POST to create one.
691-
// This is a best-effort operation: failures are reported as warnings but do
692-
// not cause the submit command to fail (the PRs are already created).
693-
func syncStack(cfg *config.Config, client github.ClientOps, s *stack.Stack) {
694-
// Collect PR numbers in stack order (bottom to top), including merged PRs.
695-
// The API expects the full list — omitting merged PRs causes a
696-
// "Stack contents have changed" rejection.
687+
// stackPRNumbers returns the PR numbers for a stack in order (bottom to top),
688+
// including merged PRs. The stacks API expects the full list — omitting merged
689+
// PRs causes a "Stack contents have changed" rejection.
690+
func stackPRNumbers(s *stack.Stack) []int {
697691
var prNumbers []int
698692
for _, b := range s.Branches {
699693
if b.PullRequest != nil {
700694
prNumbers = append(prNumbers, b.PullRequest.Number)
701695
}
702696
}
697+
return prNumbers
698+
}
699+
700+
// syncStack creates or updates a stack on GitHub from the active PRs.
701+
// If the stack already exists (s.ID is set), it calls the PUT endpoint with
702+
// the full list of PRs to keep the remote stack in sync. If no stack exists
703+
// yet, it calls POST to create one.
704+
// This is a best-effort operation: failures are reported as warnings but do
705+
// not cause the submit command to fail (the PRs are already created).
706+
//
707+
// It returns true when the remote stack object reflects the local stack
708+
// (created, updated, or already in sync) and false otherwise (fewer than two
709+
// PRs, an unresolved divergence, stacked PRs unavailable, or an API failure).
710+
func syncStack(cfg *config.Config, client github.ClientOps, s *stack.Stack) bool {
711+
prNumbers := stackPRNumbers(s)
703712

704713
// The API requires at least 2 PRs to form a stack.
705714
if len(prNumbers) < 2 {
706-
return
715+
return false
707716
}
708717

709718
if s.ID != "" {
710-
updateStack(cfg, client, s, prNumbers)
711-
return
719+
return updateStack(cfg, client, s, prNumbers)
712720
}
713721

714722
// No locally tracked stack ID. The stack may already exist on GitHub
715723
// (created from the web UI or another clone) without being recorded
716724
// locally. Adopt it instead of blindly creating a new one, which the API
717725
// rejects because the PRs are already part of a stack.
718-
if adoptRemoteStack(cfg, client, s, prNumbers) {
719-
return
726+
if handled, synced := adoptRemoteStack(cfg, client, s, prNumbers); handled {
727+
return synced
720728
}
721729

722-
createNewStack(cfg, client, s, prNumbers)
730+
return createNewStack(cfg, client, s, prNumbers)
723731
}
724732

725733
// adoptRemoteStack reconciles a locally untracked stack (s.ID == "") with the
@@ -728,16 +736,17 @@ func syncStack(cfg *config.Config, client github.ClientOps, s *stack.Stack) {
728736
// adopt that stack rather than POST a new one (which the API rejects because
729737
// the PRs are already stacked).
730738
//
731-
// It returns true when it has fully handled the sync — either by adopting and
732-
// updating the existing stack, or by intentionally refusing to modify a
733-
// divergent remote stack — and false when no matching remote stack exists and
734-
// the caller should create a new one.
735-
func adoptRemoteStack(cfg *config.Config, client github.ClientOps, s *stack.Stack, prNumbers []int) bool {
739+
// It returns (handled, synced). handled is true when it has fully handled the
740+
// sync — either by adopting and updating the existing stack, or by
741+
// intentionally refusing to modify a divergent remote stack — and false when no
742+
// matching remote stack exists and the caller should create a new one. synced
743+
// is true only when the remote stack object now reflects the local stack.
744+
func adoptRemoteStack(cfg *config.Config, client github.ClientOps, s *stack.Stack, prNumbers []int) (bool, bool) {
736745
stacks, err := client.ListStacks()
737746
if err != nil {
738747
// Couldn't inspect remote state — fall back to the create path, which
739748
// reports its own errors (handleCreate422 covers "already stacked").
740-
return false
749+
return false, false
741750
}
742751

743752
matched, err := findMatchingStack(stacks, prNumbers)
@@ -748,12 +757,12 @@ func adoptRemoteStack(cfg *config.Config, client github.ClientOps, s *stack.Stac
748757
cfg.Warningf("Your PRs belong to multiple stacks on GitHub — reconcile them before submitting")
749758
cfg.Printf(" Run `%s` to import a stack, or unstack the PRs from the web",
750759
cfg.ColorCyan("gh stack checkout <pr>"))
751-
return true
760+
return true, false
752761
}
753762

754763
if matched == nil {
755764
// No existing stack contains any of our PRs — create a new one.
756-
return false
765+
return false, false
757766
}
758767

759768
// A remote stack already contains some of our PRs. Refuse to silently drop
@@ -763,7 +772,7 @@ func adoptRemoteStack(cfg *config.Config, client github.ClientOps, s *stack.Stac
763772
formatPRList(dropped), plural(len(dropped), "is", "are"))
764773
cfg.Printf(" Run `%s` to import the full stack, then `%s`",
765774
cfg.ColorCyan("gh stack checkout <pr>"), cfg.ColorCyan("gh stack submit"))
766-
return true
775+
return true, false
767776
}
768777

769778
// Every PR in the remote stack is tracked locally (and we may have added
@@ -773,12 +782,11 @@ func adoptRemoteStack(cfg *config.Config, client github.ClientOps, s *stack.Stac
773782

774783
if slicesEqual(matched.PullRequests, prNumbers) {
775784
cfg.Successf("Linked to the existing stack on GitHub (%d PRs, already up to date)", len(prNumbers))
776-
return true
785+
return true, true
777786
}
778787

779788
cfg.Infof("Found the stack on GitHub — updating it to match your local stack")
780-
updateStack(cfg, client, s, prNumbers)
781-
return true
789+
return true, updateStack(cfg, client, s, prNumbers)
782790
}
783791

784792
// prsMissingFrom returns the numbers in remote that do not appear in local,
@@ -800,7 +808,8 @@ func prsMissingFrom(remote, local []int) []int {
800808
// updateStack calls the PUT endpoint to sync the full PR list for an existing stack.
801809
// If the remote stack was deleted (404), it clears the local ID and falls through
802810
// to createNewStack so the user doesn't need to re-run the command.
803-
func updateStack(cfg *config.Config, client github.ClientOps, s *stack.Stack, prNumbers []int) {
811+
// Returns true when the remote stack was updated (or recreated) successfully.
812+
func updateStack(cfg *config.Config, client github.ClientOps, s *stack.Stack, prNumbers []int) bool {
804813
if err := client.UpdateStack(s.ID, prNumbers); err != nil {
805814
var httpErr *api.HTTPError
806815
if errors.As(err, &httpErr) {
@@ -809,7 +818,7 @@ func updateStack(cfg *config.Config, client github.ClientOps, s *stack.Stack, pr
809818
// Stack was deleted on GitHub — clear the stale ID and
810819
// immediately try to re-create it.
811820
s.ID = ""
812-
createNewStack(cfg, client, s, prNumbers)
821+
return createNewStack(cfg, client, s, prNumbers)
813822
case 422:
814823
// A merged branch whose ref has been deleted upstream breaks the
815824
// stack's base→head chain, so the update is rejected. This is
@@ -818,7 +827,7 @@ func updateStack(cfg *config.Config, client github.ClientOps, s *stack.Stack, pr
818827
// than alarming the user with a raw API error.
819828
if strings.Contains(httpErr.Message, "must form a stack") && len(s.MergedBranches()) > 0 {
820829
cfg.Infof("Merged PRs have left the stack on GitHub, so it wasn't updated — your unmerged PRs were pushed and re-based onto the trunk")
821-
return
830+
return false
822831
}
823832
cfg.Warningf("Failed to update stack on GitHub: %s", httpErr.Message)
824833
default:
@@ -827,34 +836,38 @@ func updateStack(cfg *config.Config, client github.ClientOps, s *stack.Stack, pr
827836
} else {
828837
cfg.Warningf("Failed to update stack on GitHub: %v", err)
829838
}
830-
return
839+
return false
831840
}
832841
cfg.Successf("Stack updated on GitHub with %d PRs", len(prNumbers))
842+
return true
833843
}
834844

835845
// createNewStack calls the POST endpoint to create a new stack, handling the
836846
// three types of 422 errors the API may return.
837-
func createNewStack(cfg *config.Config, client github.ClientOps, s *stack.Stack, prNumbers []int) {
847+
// Returns true when the stack was created or is confirmed already in sync.
848+
func createNewStack(cfg *config.Config, client github.ClientOps, s *stack.Stack, prNumbers []int) bool {
838849
stackID, err := client.CreateStack(prNumbers)
839850
if err == nil {
840851
s.ID = strconv.Itoa(stackID)
841852
cfg.Successf("Stack created on GitHub with %d PRs", len(prNumbers))
842-
return
853+
return true
843854
}
844855

845856
var httpErr *api.HTTPError
846857
if !errors.As(err, &httpErr) {
847858
cfg.Warningf("Failed to create stack on GitHub: %v", err)
848-
return
859+
return false
849860
}
850861

851862
switch httpErr.StatusCode {
852863
case 422:
853-
handleCreate422(cfg, httpErr, prNumbers)
864+
return handleCreate422(cfg, httpErr, prNumbers)
854865
case 404:
855866
warnStacksUnavailableOrPAT(cfg)
867+
return false
856868
default:
857869
cfg.Warningf("Failed to create stack on GitHub: %s", httpErr.Message)
870+
return false
858871
}
859872
}
860873

@@ -863,7 +876,10 @@ func createNewStack(cfg *config.Config, client github.ClientOps, s *stack.Stack,
863876
// - "Stack must contain at least two pull requests"
864877
// - "Pull requests must form a stack, where each PR's base ref is the previous PR's head ref"
865878
// - "Pull requests #123, #124, #125 are already stacked"
866-
func handleCreate422(cfg *config.Config, httpErr *api.HTTPError, prNumbers []int) {
879+
//
880+
// Returns true only when the PRs are already stacked together (i.e. the remote
881+
// stack already matches), which counts as in sync.
882+
func handleCreate422(cfg *config.Config, httpErr *api.HTTPError, prNumbers []int) bool {
867883
msg := httpErr.Message
868884

869885
if isAlreadyStackedError(msg) {
@@ -872,22 +888,23 @@ func handleCreate422(cfg *config.Config, httpErr *api.HTTPError, prNumbers []int
872888
// If only a subset matches, the PRs are in a different stack.
873889
if allPRsInMessage(msg, prNumbers) {
874890
cfg.Successf("Stack with %d PRs is up to date", len(prNumbers))
875-
return
891+
return true
876892
}
877893
cfg.Warningf("One or more PRs are already part of a different stack on GitHub")
878894
cfg.Printf(" Run `%s` to import the existing stack, or unstack the PRs from the web",
879895
cfg.ColorCyan("gh stack checkout <pr>"))
880-
return
896+
return false
881897
}
882898

883899
if strings.Contains(msg, "must form a stack") {
884900
cfg.Warningf("Cannot create stack: %s", msg)
885901
cfg.Printf(" Each PR's base branch must match the previous PR's head branch.")
886-
return
902+
return false
887903
}
888904

889905
// "at least two" or any other validation error
890906
cfg.Warningf("Could not create stack: %s", msg)
907+
return false
891908
}
892909

893910
// allPRsInMessage checks whether every PR number in prNumbers appears

cmd/sync.go

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,13 @@ package cmd
33
import (
44
"errors"
55
"fmt"
6+
"strconv"
67
"strings"
78

89
"github.com/cli/go-gh/v2/pkg/prompter"
910
"github.com/github/gh-stack/internal/config"
1011
"github.com/github/gh-stack/internal/git"
12+
"github.com/github/gh-stack/internal/github"
1113
"github.com/github/gh-stack/internal/modify"
1214
"github.com/github/gh-stack/internal/stack"
1315
"github.com/spf13/cobra"
@@ -33,11 +35,20 @@ This command performs a safe, non-interactive synchronization:
3335
3. Cascade-rebases stack branches onto their updated parents
3436
4. Pushes all branches atomically (using --force-with-lease --atomic)
3537
5. Syncs PR state from GitHub
38+
6. Links the stack's open PRs into a stack on GitHub (creating or updating
39+
the remote stack object) when two or more PRs exist
3640
3741
If a rebase conflict is detected, all branches are restored to their
3842
original state and you are advised to run "gh stack rebase" to resolve
3943
conflicts interactively.
4044
45+
Sync never opens pull requests — use "gh stack submit" for that. It only
46+
links PRs that already exist. The final message reflects what happened:
47+
"Stack synced" means the stack object on GitHub now matches your local
48+
stack, while "Branches synced" means the branches were rebased and pushed
49+
but no remote stack object was created or updated (for example, when fewer
50+
than two PRs exist yet).
51+
4152
Use --prune to delete local branches for merged PRs. Stack metadata is
4253
preserved so that rebase and display logic continue to work correctly.
4354
If you are on a branch that would be pruned, your checkout is moved to
@@ -220,6 +231,18 @@ func runSync(cfg *config.Config, opts *syncOptions) error {
220231
cfg.Printf("Merged: %s", strings.Join(names, ", "))
221232
}
222233

234+
// --- Step 5b: Reconcile the remote stack object ---
235+
// syncStackPRs above only refreshes local PR associations; it does not touch
236+
// the stack object on GitHub. When the branches have open PRs, link them into
237+
// a stack so the remote reflects the local stack. This never opens PRs — that
238+
// is still `gh stack submit`'s job. stackSynced records whether the remote
239+
// stack object actually reflects the local stack, which determines the final
240+
// summary message below.
241+
stackSynced := false
242+
if client, err := cfg.GitHubClient(); err == nil {
243+
stackSynced = syncRemoteStack(cfg, client, s)
244+
}
245+
223246
// --- Step 6: Prune merged branches (optional) ---
224247
doPrune := opts.prune
225248
if !doPrune {
@@ -316,10 +339,55 @@ func runSync(cfg *config.Config, opts *syncOptions) error {
316339
}
317340

318341
cfg.Printf("")
319-
cfg.Successf("Stack synced")
342+
if stackSynced {
343+
cfg.Successf("Stack synced")
344+
} else {
345+
// The branches were fetched, rebased, and pushed, but no stack object on
346+
// GitHub was created or updated (no PRs, fewer than two PRs, stacked PRs
347+
// unavailable, or a divergence). Report only what actually happened.
348+
cfg.Successf("Branches synced")
349+
}
320350
return nil
321351
}
322352

353+
// syncRemoteStack reconciles the stack object on GitHub with the local stack's
354+
// open PRs. It only links existing PRs into a stack — it never opens PRs (use
355+
// `gh stack submit` for that). It returns true when the remote stack object now
356+
// reflects the local stack (created, updated, adopted, or already in sync), and
357+
// false when there is nothing to sync or the remote stack could not be
358+
// reconciled (fewer than two PRs, stacked PRs unavailable, a divergence across
359+
// multiple stacks, or an API failure).
360+
//
361+
// A stack on GitHub requires at least two open PRs, so a single-PR or PR-less
362+
// stack reconciles to false and the caller reports only the branches as synced.
363+
func syncRemoteStack(cfg *config.Config, client github.ClientOps, s *stack.Stack) bool {
364+
prNumbers := stackPRNumbers(s)
365+
if len(prNumbers) < 2 {
366+
return false
367+
}
368+
369+
// Inspect the remote stacks first so a routine sync that has not changed the
370+
// PR membership does not issue a redundant — and misleading — update.
371+
stacks, err := client.ListStacks()
372+
if err != nil {
373+
// Couldn't inspect remote state; let syncStack attempt the operation and
374+
// surface its own availability/PAT/create errors.
375+
return syncStack(cfg, client, s)
376+
}
377+
378+
if matched, mErr := findMatchingStack(stacks, prNumbers); mErr == nil &&
379+
matched != nil && slicesEqual(matched.PullRequests, prNumbers) {
380+
// The remote stack already lists exactly these PRs — record its ID so
381+
// future operations stay cheap and report it as in sync.
382+
s.ID = strconv.Itoa(matched.ID)
383+
cfg.Successf("Stack already up to date on GitHub")
384+
return true
385+
}
386+
387+
// Membership differs (or no stack exists yet): create, adopt, or update.
388+
return syncStack(cfg, client, s)
389+
}
390+
323391
// restoreBranches resets each branch to its original SHA, collecting any errors.
324392
func restoreBranches(originalRefs map[string]string) []string {
325393
var errors []string

0 commit comments

Comments
 (0)