Create/update stack on remote during sync#156
Open
skarim wants to merge 3 commits into
Open
Conversation
`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
Contributor
There was a problem hiding this comment.
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
syncRemoteStacktocmd/sync.go(short-circuits when the remote already matches, else delegates to the sharedsyncStack) and make the closing message conditional on whether the remote stack was reconciled. - Thread a
synced boolreturn throughsyncStack/createNewStack/updateStack/handleCreate422and(handled, synced)throughadoptRemoteStack; extract the sharedstackPRNumbershelper. 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
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Make
gh stack synccreate or update the stack object on GitHub when the branches already have open PRs, and report an accurate final status.gh stack synccould finish with✓ Stack syncedeven though it never created a stack on GitHub. After adopting existing branches withgh stack init, opening PRs outside the CLI, and then runninggh stack sync:There are two separate bugs here:
Sync never reconciled the remote stack object.
runSynccallssyncStackPRs, 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.The final message was unconditional.
Stack syncedwas 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:
gh stack submit's job.Stack syncedonly when the remote stack object reflects the local stack, andBranches syncedotherwise — fewer than two PRs, stacked PRs unavailable, a cross-stack divergence, or no GitHub client.Changes
cmd/sync.go:syncRemoteStackhelper: 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 tosyncStackto create/adopt/update.Stack syncedwithStack synced/Branches synceddriven by whether the remote stack object was actually synced.Longdescription to document the stack-object step and the two possible closing messages.cmd/submit.go:synced boolreturn through the existing, tested stack helpers so sync can tell whether the remote stack now matches local:syncStack,createNewStack, andupdateStackreturnbool;adoptRemoteStackreturns(handled, synced);handleCreate422returnsbool. Extract the sharedstackPRNumbershelper.cmd/sync_test.go— six new tests:TestSync_CreatesRemoteStackWhenPRsExist: open PRs, no remote stack →CreateStackis 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 →UpdateStackwith 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 syncedvsBranches synceddistinction:README.mddocs/src/content/docs/reference/cli.mdskills/gh-stack/SKILL.mddocs/src/content/docs/introduction/overview.mddocs/src/content/docs/guides/stacked-prs.mddocs/src/content/docs/guides/workflows.mdStack created with GitHub Stacks CLI • Give Feedback 💬