Skip to content

Create/update stack on remote during sync#156

Open
skarim wants to merge 3 commits into
skarim/submit-merged-stackfrom
skarim/sync-false-success
Open

Create/update stack on remote during sync#156
skarim wants to merge 3 commits into
skarim/submit-merged-stackfrom
skarim/sync-false-success

Conversation

@skarim

@skarim skarim commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Make gh stack sync create or update the stack object on GitHub when the branches already have open PRs, and report an accurate final status.

gh stack sync could finish with ✓ Stack synced even though it never created a stack on GitHub. After adopting existing branches with gh stack init, opening PRs outside the CLI, and then running gh stack sync:

Syncing PRs ...
✓ PR #10404 (jez-constexpr-version) — Open
✓ PR #10405 (jez-version) — Open

✓ Stack synced        ← but no stack was ever created on GitHub

There are two separate bugs here:

  1. Sync never reconciled the remote stack object. runSync calls syncStackPRs, which only reads PR state and links PRs to local branches — it never created or updated the stack on GitHub. So the branches were rebased and pushed and the PRs were detected, but the stack object never existed on the server.

  2. The final message was unconditional. Stack synced was printed regardless. That message is supposed to mean "the stack object on GitHub now reflects the local stack" — which can only be true when two or more open PRs exist and the remote stack was actually created/updated.

This reuses the create/adopt/update logic that already powers submit (added in #83298f1, "Adopt an existing remote stack on submit"), but handles the case the user hit: the remote stack does not necessarily exist yet.

Behavior

After syncing PR state, sync now reconciles the stack on GitHub:

  1. Links existing PRs into a stack when the stack has two or more open PRs — creating the stack if none exists, adopting an untracked one, or updating a partially-formed one. Sync never opens PRs; that is still gh stack submit's job.
  2. Skips redundant updates. If a remote stack already lists exactly these PRs, it is left untouched and reported as already up to date, so routine syncs don't issue a misleading "updated" call.
  3. Reports what actually happened. The closing message is now Stack synced only when the remote stack object reflects the local stack, and Branches synced otherwise — fewer than two PRs, stacked PRs unavailable, a cross-stack divergence, or no GitHub client.

Changes

cmd/sync.go:

  • Add a reconciliation step after PR-state sync via the new syncRemoteStack helper: it inspects existing stacks, short-circuits when one already matches (recording its ID and printing "Stack already up to date on GitHub"), and otherwise delegates to syncStack to create/adopt/update.
  • Replace the unconditional Stack synced with Stack synced / Branches synced driven by whether the remote stack object was actually synced.
  • 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 now matches local: syncStack, createNewStack, and updateStack return bool; adoptRemoteStack returns (handled, synced); handleCreate422 returns bool. Extract the shared stackPRNumbers helper.
  • Purely additive — submit's single call site ignores the new return value, so submit's behavior, output, and tests are unchanged. Reusing these helpers keeps the 404/422 handling in one tested place instead of duplicating it in sync.

cmd/sync_test.go — six new tests:

  • TestSync_CreatesRemoteStackWhenPRsExist: open PRs, no remote stack → CreateStack is called and the new ID is persisted to the stack file; output has "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".
  • 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 stack-object step and the Stack synced vs Branches synced distinction:

  • 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

Stack created with GitHub Stacks CLIGive Feedback 💬

`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
Copilot AI review requested due to automatic review settings June 29, 2026 17:32
GitHub Advanced Security started work on behalf of skarim June 29, 2026 17:32 View session
GitHub Advanced Security finished work on behalf of skarim June 29, 2026 17:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes gh stack sync so that it actually reconciles the stack object on GitHub, rather than only refreshing local PR metadata and unconditionally reporting ✓ Stack synced. After syncing PR state, sync now links the open PRs into a remote stack (creating, adopting, or updating it) whenever two or more PRs exist, and reports Stack synced only when the remote stack object truly reflects the local stack — otherwise Branches synced. It reuses the already-tested stack helpers from submit by threading a synced bool return through them, and never opens PRs (still submit's job).

Changes:

  • Add syncRemoteStack to cmd/sync.go (short-circuits when the remote already matches, else delegates to the shared syncStack) and make the closing message conditional on whether the remote stack was reconciled.
  • Thread a synced bool return through syncStack/createNewStack/updateStack/handleCreate422 and (handled, synced) through adoptRemoteStack; extract the shared stackPRNumbers helper. Submit's call site ignores the new return, so submit behavior is unchanged.
  • Add six sync tests covering create/adopt/update/fewer-than-two-PRs/unavailable/divergence, and update README, CLI reference, overview, guides, and SKILL docs.
Show a summary per file
File Description
cmd/sync.go Adds syncRemoteStack reconciliation step and Stack synced/Branches synced conditional message; new imports strconv/github.
cmd/submit.go Extracts stackPRNumbers; threads bool/(handled, synced) returns through the stack helpers (additive, submit behavior preserved).
cmd/sync_test.go Six new tests plus newSyncMockNoRebase/openPRFinder/runSyncWithGitHub helpers for remote stack reconciliation.
README.md Documents the new "Sync the stack" step and renumbers prune.
docs/src/content/docs/reference/cli.md Mirrors the CLI reference flow change.
docs/src/content/docs/introduction/overview.md Notes sync now links open PRs into a Stack.
docs/src/content/docs/guides/workflows.md Adds stack-linking as step 6.
docs/src/content/docs/guides/stacked-prs.md Adds "link open PRs into a Stack on GitHub" to the sync summary.
skills/gh-stack/SKILL.md Documents the sync stack-object step, new output lines, and Stack synced vs Branches synced.

Review details

  • Files reviewed: 9/9 changed files
  • Comments generated: 2
  • Review effort level: Medium

Comment thread cmd/sync.go
Comment thread cmd/sync.go Outdated
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.
GitHub Advanced Security started work on behalf of skarim June 29, 2026 18:29 View session
GitHub Advanced Security finished work on behalf of skarim June 29, 2026 18:30
GitHub Advanced Security started work on behalf of skarim June 29, 2026 21:10 View session
GitHub Advanced Security finished work on behalf of skarim June 29, 2026 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants