sync: retarget upstack PRs after deleting merged branches#1073
Closed
ed-irl wants to merge 1 commit into
Closed
Conversation
Add cycle detection to survivingAncestor so that if deleted branches form a cycle in their base references, the function falls back to trunk instead of looping forever.
ed-irl
changed the title
Owner
|
Thanks, @ed-irl! |
abhinav
force-pushed
the
main
branch
2 times, most recently
from
e2b9bf5 to
02c7c2d
Compare
Collaborator
Author
|
Closing in favor of #1150 |
Owner
abhinav
added a commit
that referenced
this pull request
May 23, 2026
When `gs repo sync` deletes branches whose PRs were merged, upstack PRs on the forge are left targeting deleted branches. This causes confusion on platforms like GitHub, where the PR base no longer matches the local stack state. This change retargets surviving upstack PRs on the forge to their nearest surviving ancestor (or trunk if none exists) after sync deletes merged branches. Cyclic base chains fall back to trunk to avoid infinite loops. Covered by unit tests for candidate collection and ancestor resolution, plus test scripts exercising single-merge and deep-stack scenarios end-to-end. Regarding this question on #1073: > So just to clarify/confirm the behavior: > git-spice already retargets upstack branches when their bases are deleted, but it does so entirely locally. > The focus of this change is to specifically also update the forge? > The next submit for those branches would also update the base branch in the forge. > I would appreciate some more context on what issue we're running into here. > (Context on my hesitance: repo sync is currently read-only for the forge. This would add a write, so I want to make sure we have a good reason to.) The issue encountered is branch corruption when adding the `gs bm` command - which I will post later today on this stack. Prior to the merge command, the forge itself drove merge events and the sync/submit cycle pulled merge changes and drove repointing based on merges that happened elsewhere. When I added the `gs bm` command the flow was flipped, and the stack would become corrupted if you merged anything besides the bottom branch. I'll post a ref to that PR when I post it. It took me a minute to remember what had been happening and I needed claude to help me figure it out again 😂 . It provided this somewhat more verbose explanation: > Before gs branch merge, gs read forge merge state and reconciled local — that's the read-only pattern you're describing, and repo sync was fine being one-way (forge → local). The next submit falling back to fix the forge base was fine because nothing else gs did was authoritative over forge merges. > > gs branch merge (in a separate PR in this stack) breaks that assumption. It writes merges to the forge and then reacts to those merges by mutating local state (deletes branches, reparents upstack). That makes the forge↔local invariant load-bearing inside gs bm, not just at "next submit" time. Concretely, a stack of A → B → C where A was squash-merged externally and you then run gs bm C will silently bring A's pre-squash commits back into main via B's merge, because B's remote branch still contains them and gs has no way to know B should be rebased first. > > PR #1073 is the other half of that contract: when repo sync rebases B locally, also retarget B's forge PR base so the forge and local agree on what B's parent is. Without it, gs is making local decisions (rebases, reparents) that the forge isn't told about, and the next forge-driven operation (review, or another gs bm) sees an inconsistent world. --------- Co-authored-by: Abhinav Gupta <mail@abhinavg.net>
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
2 participants
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.
Fixes a "bug" I discovered while testing my branch merge implementation where syncs could cause weird state that would mess up future merges.
repo syncdeletes merged branches, surviving upstack PRs are retargeted on the forge so their base matches the updated local state[skip changelog]: bug fix only