Create/update stack on remote during sync (#156)

* 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

* Address PR review: one ListStacks per sync, command-neutral guidance

Two follow-ups from the #156 review (both flagged optional / non-blocking).

1. Remove the redundant ListStacks round-trip on sync's create path.
   syncRemoteStack fetched the stack list for its already-up-to-date
   short-circuit, then delegated to syncStack -> adoptRemoteStack, which
   listed the stacks again — two GETs on the first-sync-create and
   membership-changed paths. Refactor adoptRemoteStack into a list-accepting
   reconcileUntrackedStack(cfg, client, s, prNumbers, stacks): syncStack now
   fetches the list once and passes it down, and syncRemoteStack reuses the
   list it already fetched. Net: exactly one ListStacks per sync. This also
   drops the (handled, synced) tuple. Submit's behavior is unchanged.

2. Make the divergence / dropped-PR guidance command-neutral. The shared
   helper emitted submit-specific wording ("reconcile them before
   submitting", "...then `gh stack submit`") that is now reachable from
   `gh stack sync`. Reword to "reconcile them first" and drop the trailing
   `gh stack submit` so it reads correctly from either command.

Tests: assert exactly one ListStacks on the create path and that the
divergence guidance is not submit-specific.

* increment skill file version

* Simplify sync reconciliation: reuse syncStack instead of a parallel path

Review feedback noted the change felt heavier than the fix warranted.
The weight came from `syncRemoteStack` (cmd/sync.go), a near-duplicate of
submit's `syncStack` — same <2-PR guard, ListStacks, and update/create
dispatch — that existed only to add an "already up to date" short-circuit.
That one optimization is what spawned the second entry point, the
pre-fetched-list threading, and the double-ListStacks it then required.

Collapse it to a single reconciliation path:

- Remove `syncRemoteStack`; `gh stack sync` now calls the shared
  `syncStack` directly. One path, one ListStacks per sync.
- Fold `createNewStack` into `reconcileUntrackedStack` (renamed from
  `adoptRemoteStack`) so it returns a single `synced bool` instead of a
  `(handled, synced)` tuple and owns its own ListStacks again.
- Inline `stackPRNumbers` back into `syncStack` (it was only extracted to
  share with the now-removed `syncRemoteStack`).
- Drop the now-unused `strconv`/`github` imports from cmd/sync.go.

Behavior note: a routine re-sync of an already-tracked stack now prints
"Stack updated on GitHub with N PRs" instead of "Stack already up to date
on GitHub". This is accurate (sync does PUT the current state) and matches
submit. The "Stack synced" / "Branches synced" summary is unchanged, and
submit's behavior is unchanged.

Author: skarim

README.md

@@ -323,7 +323,8 @@ Performs a safe, non-interactive synchronization of the entire stack:
 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
 4. **Push** — pushes all branches (uses `--force-with-lease` if a rebase occurred)
 5. **Sync PRs** — syncs PR state from GitHub and reports the status of each PR
-6. **Prune** — in interactive terminals, prompts to delete local branches for merged PRs. Use `--prune` to prune automatically
+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)
+7. **Prune** — in interactive terminals, prompts to delete local branches for merged PRs. Use `--prune` to prune automatically
 
 | Flag | Description |
 |------|-------------|

cmd/submit.go

@@ -690,7 +690,11 @@ func clearPendingModifyState(cfg *config.Config, gitDir string) {
 // yet, it calls POST to create one.
 // This is a best-effort operation: failures are reported as warnings but do
 // not cause the submit command to fail (the PRs are already created).
-func syncStack(cfg *config.Config, client github.ClientOps, s *stack.Stack) {
+//
+// It returns true when the remote stack object reflects the local stack
+// (created, updated, or already in sync) and false otherwise (fewer than two
+// PRs, an unresolved divergence, stacked PRs unavailable, or an API failure).
+func syncStack(cfg *config.Config, client github.ClientOps, s *stack.Stack) bool {
 	// Collect PR numbers in stack order (bottom to top), including merged PRs.
 	// The API expects the full list — omitting merged PRs causes a
 	// "Stack contents have changed" rejection.
@@ -703,67 +707,60 @@ func syncStack(cfg *config.Config, client github.ClientOps, s *stack.Stack) {
 
 	// The API requires at least 2 PRs to form a stack.
 	if len(prNumbers) < 2 {
-		return
+		return false
 	}
 
 	if s.ID != "" {
-		updateStack(cfg, client, s, prNumbers)
-		return
+		return updateStack(cfg, client, s, prNumbers)
 	}
 
 	// 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)
+	return reconcileUntrackedStack(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 {
+// reconcileUntrackedStack 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
+// adopt that stack rather than POST a new one (which the API rejects because the
+// PRs are already stacked). It creates a new stack when none match, refuses to
+// modify a divergent or PR-dropping stack, adopts a matching stack, or updates a
+// partially-formed one. It returns true when the remote stack object now
+// reflects the local stack.
+func reconcileUntrackedStack(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
+		return createNewStack(cfg, client, s, prNumbers)
 	}
 
 	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.Warningf("Your PRs belong to multiple stacks on GitHub — reconcile them first")
 		cfg.Printf("  Run `%s` to import a stack, or unstack the PRs from the web",
 			cfg.ColorCyan("gh stack checkout <pr>"))
-		return true
+		return false
 	}
 
 	if matched == nil {
 		// No existing stack contains any of our PRs — create a new one.
-		return false
+		return createNewStack(cfg, client, s, prNumbers)
 	}
 
 	// 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
+		cfg.Printf("  Run `%s` to import the full stack",
+			cfg.ColorCyan("gh stack checkout <pr>"))
+		return false
 	}
 
 	// Every PR in the remote stack is tracked locally (and we may have added
@@ -777,8 +774,7 @@ func adoptRemoteStack(cfg *config.Config, client github.ClientOps, s *stack.Stac
 	}
 
 	cfg.Infof("Found the stack on GitHub — updating it to match your local stack")
-	updateStack(cfg, client, s, prNumbers)
-	return true
+	return updateStack(cfg, client, s, prNumbers)
 }
 
 // prsMissingFrom returns the numbers in remote that do not appear in local,
@@ -800,7 +796,8 @@ func prsMissingFrom(remote, local []int) []int {
 // updateStack calls the PUT endpoint to sync the full PR list for an existing stack.
 // If the remote stack was deleted (404), it clears the local ID and falls through
 // to createNewStack so the user doesn't need to re-run the command.
-func updateStack(cfg *config.Config, client github.ClientOps, s *stack.Stack, prNumbers []int) {
+// Returns true when the remote stack was updated (or recreated) successfully.
+func updateStack(cfg *config.Config, client github.ClientOps, s *stack.Stack, prNumbers []int) bool {
 	if err := client.UpdateStack(s.ID, prNumbers); err != nil {
 		var httpErr *api.HTTPError
 		if errors.As(err, &httpErr) {
@@ -809,7 +806,7 @@ func updateStack(cfg *config.Config, client github.ClientOps, s *stack.Stack, pr
 				// Stack was deleted on GitHub — clear the stale ID and
 				// immediately try to re-create it.
 				s.ID = ""
-				createNewStack(cfg, client, s, prNumbers)
+				return createNewStack(cfg, client, s, prNumbers)
 			case 422:
 				// A merged branch whose ref has been deleted upstream breaks the
 				// stack's base→head chain, so the update is rejected. This is
@@ -818,7 +815,7 @@ func updateStack(cfg *config.Config, client github.ClientOps, s *stack.Stack, pr
 				// than alarming the user with a raw API error.
 				if strings.Contains(httpErr.Message, "must form a stack") && len(s.MergedBranches()) > 0 {
 					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")
-					return
+					return false
 				}
 				cfg.Warningf("Failed to update stack on GitHub: %s", httpErr.Message)
 			default:
@@ -827,34 +824,38 @@ func updateStack(cfg *config.Config, client github.ClientOps, s *stack.Stack, pr
 		} else {
 			cfg.Warningf("Failed to update stack on GitHub: %v", err)
 		}
-		return
+		return false
 	}
 	cfg.Successf("Stack updated on GitHub with %d PRs", len(prNumbers))
+	return true
 }
 
 // createNewStack calls the POST endpoint to create a new stack, handling the
 // three types of 422 errors the API may return.
-func createNewStack(cfg *config.Config, client github.ClientOps, s *stack.Stack, prNumbers []int) {
+// Returns true when the stack was created or is confirmed already in sync.
+func createNewStack(cfg *config.Config, client github.ClientOps, s *stack.Stack, prNumbers []int) bool {
 	stackID, err := client.CreateStack(prNumbers)
 	if err == nil {
 		s.ID = strconv.Itoa(stackID)
 		cfg.Successf("Stack created on GitHub with %d PRs", len(prNumbers))
-		return
+		return true
 	}
 
 	var httpErr *api.HTTPError
 	if !errors.As(err, &httpErr) {
 		cfg.Warningf("Failed to create stack on GitHub: %v", err)
-		return
+		return false
 	}
 
 	switch httpErr.StatusCode {
 	case 422:
-		handleCreate422(cfg, httpErr, prNumbers)
+		return handleCreate422(cfg, httpErr, prNumbers)
 	case 404:
 		warnStacksUnavailableOrPAT(cfg)
+		return false
 	default:
 		cfg.Warningf("Failed to create stack on GitHub: %s", httpErr.Message)
+		return false
 	}
 }
 
@@ -863,7 +864,10 @@ func createNewStack(cfg *config.Config, client github.ClientOps, s *stack.Stack,
 //   - "Stack must contain at least two pull requests"
 //   - "Pull requests must form a stack, where each PR's base ref is the previous PR's head ref"
 //   - "Pull requests #123, #124, #125 are already stacked"
-func handleCreate422(cfg *config.Config, httpErr *api.HTTPError, prNumbers []int) {
+//
+// Returns true only when the PRs are already stacked together (i.e. the remote
+// stack already matches), which counts as in sync.
+func handleCreate422(cfg *config.Config, httpErr *api.HTTPError, prNumbers []int) bool {
 	msg := httpErr.Message
 
 	if isAlreadyStackedError(msg) {
@@ -872,22 +876,23 @@ func handleCreate422(cfg *config.Config, httpErr *api.HTTPError, prNumbers []int
 		// If only a subset matches, the PRs are in a different stack.
 		if allPRsInMessage(msg, prNumbers) {
 			cfg.Successf("Stack with %d PRs is up to date", len(prNumbers))
-			return
+			return true
 		}
 		cfg.Warningf("One or more PRs are already part of a different stack on GitHub")
 		cfg.Printf("  Run `%s` to import the existing stack, or unstack the PRs from the web",
 			cfg.ColorCyan("gh stack checkout <pr>"))
-		return
+		return false
 	}
 
 	if strings.Contains(msg, "must form a stack") {
 		cfg.Warningf("Cannot create stack: %s", msg)
 		cfg.Printf("  Each PR's base branch must match the previous PR's head branch.")
-		return
+		return false
 	}
 
 	// "at least two" or any other validation error
 	cfg.Warningf("Could not create stack: %s", msg)
+	return false
 }
 
 // allPRsInMessage checks whether every PR number in prNumbers appears

cmd/sync.go

@@ -33,11 +33,20 @@ This command performs a safe, non-interactive synchronization:
   3. Cascade-rebases stack branches onto their updated parents
   4. Pushes all branches atomically (using --force-with-lease --atomic)
   5. Syncs PR state from GitHub
+  6. Links the stack's open PRs into a stack on GitHub (creating or updating
+     the remote stack object) when two or more PRs exist
 
 If a rebase conflict is detected, all branches are restored to their
 original state and you are advised to run "gh stack rebase" to resolve
 conflicts interactively.
 
+Sync never opens pull requests — use "gh stack submit" for that. It only
+links PRs that already exist. The final message reflects what happened:
+"Stack synced" means the stack object on GitHub now matches your local
+stack, while "Branches synced" means the branches were rebased and pushed
+but no remote stack object was created or updated (for example, when fewer
+than two PRs exist yet).
+
 Use --prune to delete local branches for merged PRs. Stack metadata is
 preserved so that rebase and display logic continue to work correctly.
 If you are on a branch that would be pruned, your checkout is moved to
@@ -220,6 +229,18 @@ func runSync(cfg *config.Config, opts *syncOptions) error {
 		cfg.Printf("Merged: %s", strings.Join(names, ", "))
 	}
 
+	// --- Step 5b: Reconcile the remote stack object ---
+	// syncStackPRs above only refreshes local PR associations; it does not touch
+	// the stack object on GitHub. When the branches have open PRs, link them into
+	// a stack so the remote reflects the local stack. This never opens PRs — that
+	// is still `gh stack submit`'s job. stackSynced records whether the remote
+	// stack object actually reflects the local stack, which determines the final
+	// summary message below.
+	stackSynced := false
+	if client, err := cfg.GitHubClient(); err == nil {
+		stackSynced = syncStack(cfg, client, s)
+	}
+
 	// --- Step 6: Prune merged branches (optional) ---
 	doPrune := opts.prune
 	if !doPrune {
@@ -316,7 +337,14 @@ func runSync(cfg *config.Config, opts *syncOptions) error {
 	}
 
 	cfg.Printf("")
-	cfg.Successf("Stack synced")
+	if stackSynced {
+		cfg.Successf("Stack synced")
+	} else {
+		// The branches were fetched, rebased, and pushed, but no stack object on
+		// GitHub was created or updated (no PRs, fewer than two PRs, stacked PRs
+		// unavailable, or a divergence). Report only what actually happened.
+		cfg.Successf("Branches synced")
+	}
 	return nil
 }