diff --git a/README.md b/README.md index 77ed874..7b47ac0 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,10 @@ The catch? Managing these stacks by hand is tedious. When `main` updates, you ne ## Installation -Requires [GitHub CLI][] (`gh`) installed and authenticated. +Requires: + +- [GitHub CLI][] (`gh`) installed and authenticated +- Git 2.38 or newer (for `git rebase --update-refs` support; macOS Command Line Tools and current Linux distributions all ship a compatible version) ```bash gh extension install boneskull/gh-stack @@ -278,13 +281,14 @@ If a rebase conflict occurs, resolve it and run `gh stack continue`. | Flag | Description | | --------------------- | ---------------------------------------------------------------------- | -| `-D, --dry-run` | Show what would happen without doing it | -| `-f, --from [branch]` | Submit from this branch toward leaves (bare `--from` = current branch) | -| `-c, --current` | Only submit the current branch, not descendants | -| `-u, --update` | Only update existing PRs, don't create new ones | -| `-s, --skip-prs` | Skip PR creation/update, only restack and push | -| `-y, --yes` | Skip interactive prompts; use auto-generated PR title/description | -| `--web` | Open created/updated PRs in web browser | +| `-D, --dry-run` | Show what would happen without doing it | +| `-f, --from [branch]` | Submit from this branch toward leaves (bare `--from` = current branch) | +| `-c, --current` | Only submit the current branch, not descendants | +| `-u, --update` | Only update existing PRs, don't create new ones | +| `-s, --skip-prs` | Skip PR creation/update, only restack and push | +| `-y, --yes` | Skip interactive prompts; use auto-generated PR title/description | +| `--web` | Open created/updated PRs in web browser | +| `--no-update-refs` | Do not pass `--update-refs` to git (preserves untracked bookmark branches) | ### restack @@ -296,11 +300,12 @@ If a rebase conflict occurs, resolve it and run `gh stack continue`. #### restack Flags -| Flag | Description | -| ----------------- | -------------------------------------------------------- | -| `-c, --current` | Only restack current branch, not descendants | -| `-D, --dry-run` | Show what would be done | -| `-w, --worktrees` | Rebase branches checked out in linked worktrees in-place | +| Flag | Description | +| -------------------- | -------------------------------------------------------------------------- | +| `-c, --current` | Only restack current branch, not descendants | +| `-D, --dry-run` | Show what would be done | +| `-w, --worktrees` | Rebase branches checked out in linked worktrees in-place | +| `--no-update-refs` | Do not pass `--update-refs` to git (preserves untracked bookmark branches) | ### continue @@ -324,9 +329,10 @@ This is the command to run when upstream changes have occurred (e.g., a PR in yo | Flag | Description | | ----------------- | -------------------------------------------------------- | -| `--no-restack` | Skip restacking branches (might not work well!) | -| `-D, --dry-run` | Show what would be done | -| `-w, --worktrees` | Rebase branches checked out in linked worktrees in-place | +| `--no-restack` | Skip restacking branches (might not work well!) | +| `-D, --dry-run` | Show what would be done | +| `-w, --worktrees` | Rebase branches checked out in linked worktrees in-place | +| `--no-update-refs` | Do not pass `--update-refs` to git (preserves untracked bookmark branches) | ### undo @@ -371,6 +377,10 @@ If a rebase conflict occurs in a worktree branch, **gh-stack** will tell you whi > > The `--worktrees` flag is opt-in. Without it, **gh-stack** behaves exactly as before. If none of your stack branches are checked out in linked worktrees, the flag is a harmless no-op. +> [!NOTE] +> +> When linked worktrees are detected, **gh-stack** automatically passes `--no-update-refs` to git. This prevents silent stack corruption: git's `--update-refs` silently skips any ref that is checked out in a linked worktree, which would leave the chain in a broken state. If no branches are checked out in linked worktrees (even with `--worktrees` set), **gh-stack** passes `--update-refs` normally so that any untracked bookmark branches pointing into the stack are kept in sync. + ## How It Works **gh-stack** stores metadata in your local `.git/config`: diff --git a/cmd/continue.go b/cmd/continue.go index fc6b052..be866f0 100644 --- a/cmd/continue.go +++ b/cmd/continue.go @@ -97,13 +97,14 @@ func runContinue(cmd *cobra.Command, args []string) error { _ = state.Remove(g.GetGitDir()) //nolint:errcheck // cleanup if restackErr := doRestackWithState(g, cfg, branches, RestackOptions{ - Operation: st.Operation, - UpdateOnly: st.UpdateOnly, - OpenWeb: st.Web, - PushOnly: st.PushOnly, - Branches: st.Branches, - StashRef: st.StashRef, - Worktrees: st.Worktrees, + Operation: st.Operation, + UpdateOnly: st.UpdateOnly, + OpenWeb: st.Web, + PushOnly: st.PushOnly, + Branches: st.Branches, + StashRef: st.StashRef, + Worktrees: st.Worktrees, + NoUpdateRefs: !st.UpdateRefs, }, s); restackErr != nil { // Stash handling is done by doRestackWithState (conflict saves in state, errors restore) if !errors.Is(restackErr, ErrConflict) && st.StashRef != "" { diff --git a/cmd/restack.go b/cmd/restack.go index 31fba25..9d230d6 100644 --- a/cmd/restack.go +++ b/cmd/restack.go @@ -27,15 +27,17 @@ var restackCmd = &cobra.Command{ } var ( - restackOnlyFlag bool - restackDryRunFlag bool - restackWorktreesFlag bool + restackOnlyFlag bool + restackDryRunFlag bool + restackWorktreesFlag bool + restackNoUpdateRefsFlag bool ) func init() { restackCmd.Flags().BoolVarP(&restackOnlyFlag, "current", "c", false, "only restack current branch, not descendants") restackCmd.Flags().BoolVarP(&restackDryRunFlag, "dry-run", "D", false, "show what would be done") restackCmd.Flags().BoolVarP(&restackWorktreesFlag, "worktrees", "w", false, "rebase branches checked out in linked worktrees in-place") + restackCmd.Flags().BoolVar(&restackNoUpdateRefsFlag, "no-update-refs", false, "do not pass --update-refs to git (preserves untracked bookmark branches pointing into the stack)") rootCmd.AddCommand(restackCmd) } @@ -104,10 +106,11 @@ func runRestack(cmd *cobra.Command, args []string) error { } err = doRestackWithState(g, cfg, branches, RestackOptions{ - DryRun: restackDryRunFlag, - Operation: state.OperationRestack, - StashRef: stashRef, - Worktrees: worktrees, + DryRun: restackDryRunFlag, + Operation: state.OperationRestack, + StashRef: stashRef, + Worktrees: worktrees, + NoUpdateRefs: restackNoUpdateRefsFlag, }, s) // Restore auto-stashed changes after operation (unless conflict, which saves stash in state) @@ -150,6 +153,11 @@ type RestackOptions struct { // present in the map are rebased directly in their worktree directory instead // of being checked out in the main working tree. Worktrees map[string]string + // NoUpdateRefs suppresses --update-refs on rebase invocations when true. + // --update-refs is also suppressed automatically when Worktrees is non-empty + // because git silently skips refs checked out in other worktrees, which + // would corrupt the stack without any error or warning. + NoUpdateRefs bool } // doRestackWithState performs restack and saves state with the given operation type. @@ -163,6 +171,14 @@ func doRestackWithState(g *git.Git, cfg *config.Config, branches []*tree.Node, o return err } + // Resolve the effective --update-refs setting once for this operation. + // We always pass the flag explicitly (either --update-refs or --no-update-refs) + // to override any ambient rebase.updateRefs git config setting. + // Suppress when linked worktrees are detected (Worktrees map non-empty): + // git silently skips refs that are checked out in another worktree rather + // than refusing, which would leave the stack in a broken state with no error. + updateRefs := !opts.NoUpdateRefs && len(opts.Worktrees) == 0 + for i, b := range branches { parent, err := cfg.GetParent(b.Name) if err != nil { @@ -254,13 +270,15 @@ func doRestackWithState(g *git.Git, cfg *config.Config, branches []*tree.Node, o var rebaseErr error if wtPath != "" { - // Branch is checked out in a linked worktree -- rebase there directly + // Branch is checked out in a linked worktree -- rebase there directly. + // updateRefs is already false when worktrees are active (see resolution + // rule above), so RebaseHere/RebaseOntoHere pass --no-update-refs. fmt.Printf(" %s\n", s.Muted(fmt.Sprintf("Using worktree at %s for %s", wtPath, b.Name))) gitWt := git.New(wtPath) if useOnto { - rebaseErr = gitWt.RebaseOntoHere(parent, storedForkPoint) + rebaseErr = gitWt.RebaseOntoHere(parent, storedForkPoint, updateRefs) } else { - rebaseErr = gitWt.RebaseHere(parent) + rebaseErr = gitWt.RebaseHere(parent, updateRefs) } // If git failed for a non-conflict reason (e.g. worktree dir was removed), // wrap the error with context so the user knows which worktree we tried. @@ -274,9 +292,9 @@ func doRestackWithState(g *git.Git, cfg *config.Config, branches []*tree.Node, o } if useOnto { - rebaseErr = g.RebaseOnto(parent, storedForkPoint, b.Name) + rebaseErr = g.RebaseOnto(parent, storedForkPoint, b.Name, updateRefs) } else { - rebaseErr = g.Rebase(parent) + rebaseErr = g.Rebase(parent, updateRefs) } } @@ -298,6 +316,7 @@ func doRestackWithState(g *git.Git, cfg *config.Config, branches []*tree.Node, o Branches: opts.Branches, StashRef: opts.StashRef, Worktrees: opts.Worktrees, + UpdateRefs: updateRefs, } _ = state.Save(g.GetGitDir(), st) //nolint:errcheck // best effort - user can recover manually diff --git a/cmd/submit.go b/cmd/submit.go index 4063c80..6738bf7 100644 --- a/cmd/submit.go +++ b/cmd/submit.go @@ -38,13 +38,14 @@ If a rebase conflict occurs, resolve it and run 'gh stack continue'.`, } var ( - submitDryRunFlag bool - submitCurrentOnlyFlag bool - submitUpdateOnlyFlag bool - submitPushOnlyFlag bool - submitYesFlag bool - submitWebFlag bool - submitFromFlag string + submitDryRunFlag bool + submitCurrentOnlyFlag bool + submitUpdateOnlyFlag bool + submitPushOnlyFlag bool + submitYesFlag bool + submitWebFlag bool + submitFromFlag string + submitNoUpdateRefsFlag bool ) // prAction describes what we will do for a branch in the PR phase after push. @@ -86,6 +87,7 @@ func init() { submitCmd.Flags().BoolVar(&submitWebFlag, "web", false, "open created/updated PRs in web browser") submitCmd.Flags().StringVarP(&submitFromFlag, "from", "f", "", "submit from this branch toward leaves (default: entire stack; bare --from = current branch)") submitCmd.Flags().Lookup("from").NoOptDefVal = "HEAD" + submitCmd.Flags().BoolVar(&submitNoUpdateRefsFlag, "no-update-refs", false, "do not pass --update-refs to git (preserves untracked bookmark branches pointing into the stack)") rootCmd.AddCommand(submitCmd) } @@ -205,13 +207,14 @@ func runSubmit(cmd *cobra.Command, args []string) error { // Phase 1: Restack fmt.Println(s.Bold("=== Phase 1: Restack ===")) if restackErr := doRestackWithState(g, cfg, branches, RestackOptions{ - DryRun: submitDryRunFlag, - Operation: state.OperationSubmit, - UpdateOnly: submitUpdateOnlyFlag, - OpenWeb: submitWebFlag, - PushOnly: submitPushOnlyFlag, - Branches: branchNames, - StashRef: stashRef, + DryRun: submitDryRunFlag, + Operation: state.OperationSubmit, + UpdateOnly: submitUpdateOnlyFlag, + OpenWeb: submitWebFlag, + PushOnly: submitPushOnlyFlag, + Branches: branchNames, + StashRef: stashRef, + NoUpdateRefs: submitNoUpdateRefsFlag, }, s); restackErr != nil { // Stash is saved in state for conflicts; restore on other errors if !errors.Is(restackErr, ErrConflict) && stashRef != "" { diff --git a/cmd/sync.go b/cmd/sync.go index aa40ee5..9389a6b 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -25,15 +25,17 @@ var syncCmd = &cobra.Command{ } var ( - syncNoRestackFlag bool - syncDryRunFlag bool - syncWorktreesFlag bool + syncNoRestackFlag bool + syncDryRunFlag bool + syncWorktreesFlag bool + syncNoUpdateRefsFlag bool ) func init() { syncCmd.Flags().BoolVar(&syncNoRestackFlag, "no-restack", false, "skip restacking branches") syncCmd.Flags().BoolVarP(&syncDryRunFlag, "dry-run", "D", false, "show what would be done") syncCmd.Flags().BoolVarP(&syncWorktreesFlag, "worktrees", "w", false, "rebase branches checked out in linked worktrees in-place") + syncCmd.Flags().BoolVar(&syncNoUpdateRefsFlag, "no-update-refs", false, "do not pass --update-refs to git (preserves untracked bookmark branches pointing into the stack)") rootCmd.AddCommand(syncCmd) } @@ -255,6 +257,18 @@ func runSync(cmd *cobra.Command, args []string) error { } } + // Build worktree map once, used by both the retarget rebase and the main + // restack loop so both apply the same suppression rule: suppress + // --update-refs when any worktrees are active (len > 0). + var worktrees map[string]string + if syncWorktreesFlag { + var wtErr error + worktrees, wtErr = g.ListWorktrees() + if wtErr != nil { + return fmt.Errorf("failed to list worktrees: %w", wtErr) + } + } + // Handle merged branches root, _ := tree.Build(cfg) //nolint:errcheck // nil root is fine, FindNode handles it @@ -325,14 +339,17 @@ func runSync(cmd *cobra.Command, args []string) error { } } - // Rebase using --onto if we have a fork point + // Rebase using --onto if we have a fork point. + // Suppress --update-refs when --worktrees is active (same rule as the + // main restack loop) or when the user passed --no-update-refs. if rt.forkPoint != "" && g.CommitExists(rt.forkPoint) { displayForkPoint := rt.forkPoint if len(displayForkPoint) > 8 { displayForkPoint = displayForkPoint[:8] } + retargetUpdateRefs := !syncNoUpdateRefsFlag && len(worktrees) == 0 fmt.Printf("Rebasing %s onto %s (from fork point %s)...\n", s.Branch(rt.childName), s.Branch(trunk), displayForkPoint) - if rebaseErr := g.RebaseOnto(trunk, rt.forkPoint, rt.childName); rebaseErr != nil { + if rebaseErr := g.RebaseOnto(trunk, rt.forkPoint, rt.childName, retargetUpdateRefs); rebaseErr != nil { fmt.Printf("%s --onto rebase failed, will try normal restack: %v\n", s.WarningIcon(), rebaseErr) // Don't return error - let restack try } else { @@ -361,25 +378,16 @@ func runSync(cmd *cobra.Command, args []string) error { return err } - // Build worktree map if --worktrees flag is set - var worktrees map[string]string - if syncWorktreesFlag { - var wtErr error - worktrees, wtErr = g.ListWorktrees() - if wtErr != nil { - return fmt.Errorf("failed to list worktrees: %w", wtErr) - } - } - // Restack from trunk's children for _, child := range root.Children { allBranches := []*tree.Node{child} allBranches = append(allBranches, tree.GetDescendants(child)...) if err := doRestackWithState(g, cfg, allBranches, RestackOptions{ - DryRun: syncDryRunFlag, - Operation: state.OperationRestack, - StashRef: stashRef, - Worktrees: worktrees, + DryRun: syncDryRunFlag, + Operation: state.OperationRestack, + StashRef: stashRef, + Worktrees: worktrees, + NoUpdateRefs: syncNoUpdateRefsFlag, }, s); err != nil { if errors.Is(err, ErrConflict) { hitConflict = true diff --git a/e2e/restack_test.go b/e2e/restack_test.go index 95595d2..0a78173 100644 --- a/e2e/restack_test.go +++ b/e2e/restack_test.go @@ -290,3 +290,79 @@ func TestRestackOntoUsedForRewrittenParent(t *testing.T) { env.AssertAncestor("feature-a", "feature-b") env.AssertNoRebaseInProgress() } + +// TestRestackMovesUntrackedBookmark verifies that gh-stack restack (with the +// default --update-refs) moves untracked bookmark branches that point into the +// rebased chain. +func TestRestackMovesUntrackedBookmark(t *testing.T) { + env := NewTestEnv(t) + env.MustRun("init") + + env.MustRun("create", "feature-a") + env.CreateCommit("feature a work") + + env.MustRun("create", "feature-b") + env.CreateCommit("feature b work") + + // Record feature-a's tip before restack + aBeforeRestack := env.BranchTip("feature-a") + + // Create an untracked bookmark pointing at feature-a + env.Git("branch", "my-bookmark", "feature-a") + + // Move main forward so a restack is required + env.Git("checkout", "main") + env.CreateCommit("main moved forward") + + // Restack from feature-a (default: --update-refs active) + env.Git("checkout", "feature-a") + env.MustRun("restack") + + aAfterRestack := env.BranchTip("feature-a") + bookmarkAfter := env.BranchTip("my-bookmark") + + if aAfterRestack == aBeforeRestack { + t.Fatal("expected feature-a to be rebased, but its tip did not change") + } + if bookmarkAfter != aAfterRestack { + t.Errorf("expected my-bookmark to track new feature-a tip %s after --update-refs, got %s", + aAfterRestack[:8], bookmarkAfter[:8]) + } +} + +// TestRestackNoUpdateRefsFlagPreservesBookmark verifies that --no-update-refs +// leaves untracked bookmark branches pointing at their original commits. +func TestRestackNoUpdateRefsFlagPreservesBookmark(t *testing.T) { + env := NewTestEnv(t) + env.MustRun("init") + + env.MustRun("create", "feature-a") + env.CreateCommit("feature a work") + + env.MustRun("create", "feature-b") + env.CreateCommit("feature b work") + + // Record feature-a's tip and create an untracked bookmark there + aBeforeRestack := env.BranchTip("feature-a") + env.Git("branch", "my-bookmark", "feature-a") + bookmarkBefore := env.BranchTip("my-bookmark") + + // Move main forward so a restack is required + env.Git("checkout", "main") + env.CreateCommit("main moved forward") + + // Restack with --no-update-refs; bookmark must not move + env.Git("checkout", "feature-a") + env.MustRun("restack", "--no-update-refs") + + aAfterRestack := env.BranchTip("feature-a") + bookmarkAfter := env.BranchTip("my-bookmark") + + if aAfterRestack == aBeforeRestack { + t.Fatal("expected feature-a to be rebased, but its tip did not change") + } + if bookmarkAfter != bookmarkBefore { + t.Errorf("expected my-bookmark to be unchanged with --no-update-refs: was %s, now %s", + bookmarkBefore[:8], bookmarkAfter[:8]) + } +} diff --git a/e2e/worktree_test.go b/e2e/worktree_test.go index f0ee38f..2365084 100644 --- a/e2e/worktree_test.go +++ b/e2e/worktree_test.go @@ -220,3 +220,70 @@ func containsString(s, substr string) bool { } return false } + +// TestRestackWorktreesSuppressesUpdateRefs verifies that --worktrees mode +// automatically suppresses --update-refs. When an intermediate branch is +// checked out in a linked worktree, git silently skips updating that ref with +// --update-refs, which leaves the stack in a broken state. gh-stack prevents +// this by always passing --no-update-refs when --worktrees is active. +// +// We verify the suppression indirectly: an untracked bookmark pointing at the +// worktree branch must NOT be moved after the restack (proving --no-update-refs +// was passed). If it were moved, git would have had to process the worktree +// branch's ref — which we know it silently skips — and the stack would be +// broken. +func TestRestackWorktreesSuppressesUpdateRefs(t *testing.T) { + env := NewTestEnv(t) + env.MustRun("init") + + // Build chain: main -> feature-a -> feature-b -> feature-c + env.MustRun("create", "feature-a") + env.CreateCommit("feature a work") + + env.MustRun("create", "feature-b") + env.CreateCommit("feature b work") + + env.MustRun("create", "feature-c") + env.CreateCommit("feature c work") + + // Check out feature-b in a linked worktree + wtPath := filepath.Join(t.TempDir(), "wt-feature-b") + env.CreateWorktree("feature-b", wtPath) + + // Create an untracked bookmark pointing at feature-b + env.Git("branch", "bookmark", "feature-b") + bookmarkBefore := env.BranchTip("bookmark") + + // Move main forward so a restack is needed + env.Git("checkout", "main") + env.CreateCommit("main moved forward") + + // Restack with --worktrees; --update-refs must be suppressed automatically + env.Git("checkout", "feature-a") + env.MustRun("restack", "--worktrees") + + // Stack integrity: all branches must be in correct ancestry order + env.AssertAncestor("main", "feature-a") + env.AssertAncestor("feature-a", "feature-b") + env.AssertAncestor("feature-b", "feature-c") + env.AssertNoRebaseInProgress() + + // Verify the worktree branch is intact (reachable from feature-c) + wtBranchTip := env.BranchTip("feature-b") + mainTip := env.BranchTip("main") + if wtBranchTip == mainTip { + t.Error("feature-b should not point at main after worktree restack") + } + + // The bookmark must NOT have moved: --no-update-refs was in effect. + // If it moved, it would prove --update-refs was passed, and the stack + // integrity check above would likely also fail (silent skip corrupts the chain). + bookmarkAfter := env.BranchTip("bookmark") + if bookmarkAfter != bookmarkBefore { + t.Errorf("bookmark moved from %s to %s under --worktrees: --no-update-refs was not suppressed", + bookmarkBefore[:8], bookmarkAfter[:8]) + } + + // Clean up + _ = os.RemoveAll(wtPath) +} diff --git a/internal/git/git.go b/internal/git/git.go index 8a89c66..17c9efe 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -31,6 +31,44 @@ func resolveGitPath() (string, error) { return gitPath, errGitPath } +var ( + gitVersionOnce sync.Once + gitVersionMaj int + gitVersionMin int +) + +// initGitVersion lazily resolves and caches the major.minor of the installed git. +// On parse failure (unusual version strings) both values remain zero. +func initGitVersion() { + gitVersionOnce.Do(func() { + p, err := resolveGitPath() + if err != nil { + return + } + out, err := exec.Command(p, "--version").Output() + if err != nil { + return + } + // Output: "git version X.Y.Z[.extra]" + fmt.Sscanf(strings.TrimSpace(string(out)), "git version %d.%d", &gitVersionMaj, &gitVersionMin) //nolint:errcheck // best effort + }) +} + +// requireUpdateRefsSupport returns an error if the installed git is older than +// 2.38, which introduced --update-refs. When the version cannot be determined +// (unusual build) the check is skipped and git itself will surface any failure. +func requireUpdateRefsSupport() error { + initGitVersion() + if gitVersionMaj == 0 { + return nil // version unknown; proceed and let git surface the error + } + if gitVersionMaj > 2 || (gitVersionMaj == 2 && gitVersionMin >= 38) { + return nil + } + return fmt.Errorf("git 2.38 or newer is required (installed: %d.%d); upgrade git and try again", + gitVersionMaj, gitVersionMin) +} + // Git provides git operations for a repository. type Git struct { repoPath string @@ -167,9 +205,27 @@ func (g *Git) NeedsRebase(branch, parent string) (bool, error) { return mergeBase != parentTip, nil } +// updateRefsArg returns the --update-refs or --no-update-refs flag string. +// Passing the explicit flag ensures we override any ambient rebase.updateRefs +// git config setting, making behavior predictable regardless of user config. +func updateRefsArg(updateRefs bool) string { + if updateRefs { + return "--update-refs" + } + return "--no-update-refs" +} + // Rebase rebases the current branch onto target. -func (g *Git) Rebase(onto string) error { - return g.runInteractive("rebase", onto) +// updateRefs controls whether --update-refs is passed to git rebase. +// When true, git moves any branch refs that point to commits inside the rebased +// range. When false, --no-update-refs is passed explicitly to override any +// ambient rebase.updateRefs git config. +// Both flags require Git 2.38+; an error is returned if the requirement is not met. +func (g *Git) Rebase(onto string, updateRefs bool) error { + if err := requireUpdateRefsSupport(); err != nil { + return err + } + return g.runInteractive("rebase", updateRefsArg(updateRefs), onto) } // CommitExists checks if a commit SHA exists in the repository. @@ -254,12 +310,17 @@ func (g *Git) ListRemoteBranches() (map[string]bool, error) { // Checks out the branch first, then runs: git rebase --onto // Useful when a parent branch was squash-merged and we need to replay only // the commits unique to the child branch. -func (g *Git) RebaseOnto(newBase, oldBase, branch string) error { +// updateRefs controls whether --update-refs is passed; see Rebase for details. +// Requires Git 2.38+; an error is returned if the requirement is not met. +func (g *Git) RebaseOnto(newBase, oldBase, branch string, updateRefs bool) error { + if err := requireUpdateRefsSupport(); err != nil { + return err + } // Checkout the branch to rebase (git rebase operates on HEAD) if err := g.Checkout(branch); err != nil { return err } - return g.runInteractive("rebase", "--onto", newBase, oldBase) + return g.runInteractive("rebase", updateRefsArg(updateRefs), "--onto", newBase, oldBase) } // RebaseContinue continues an in-progress rebase. @@ -354,15 +415,25 @@ func (g *Git) ListWorktrees() (map[string]string, error) { // This is semantically identical to Rebase but named explicitly to clarify // that no checkout is needed -- the caller is operating inside a worktree // that already has the target branch checked out. -func (g *Git) RebaseHere(onto string) error { - return g.runInteractive("rebase", onto) +// updateRefs controls whether --update-refs is passed; see Rebase for details. +// Requires Git 2.38+; an error is returned if the requirement is not met. +func (g *Git) RebaseHere(onto string, updateRefs bool) error { + if err := requireUpdateRefsSupport(); err != nil { + return err + } + return g.runInteractive("rebase", updateRefsArg(updateRefs), onto) } // RebaseOntoHere runs `git rebase --onto ` on the // already-checked-out HEAD. Needed for fork-point rebases in worktrees // where we cannot (and don't need to) checkout first. -func (g *Git) RebaseOntoHere(newBase, oldBase string) error { - return g.runInteractive("rebase", "--onto", newBase, oldBase) +// updateRefs controls whether --update-refs is passed; see Rebase for details. +// Requires Git 2.38+; an error is returned if the requirement is not met. +func (g *Git) RebaseOntoHere(newBase, oldBase string, updateRefs bool) error { + if err := requireUpdateRefsSupport(); err != nil { + return err + } + return g.runInteractive("rebase", updateRefsArg(updateRefs), "--onto", newBase, oldBase) } // GetGitDir returns the .git directory path. diff --git a/internal/git/git_test.go b/internal/git/git_test.go index 8c7ddf7..d68b2e9 100644 --- a/internal/git/git_test.go +++ b/internal/git/git_test.go @@ -457,7 +457,7 @@ func TestRebaseOnto(t *testing.T) { // Now rebase child onto trunk, using parent tip as the fork point // This should only replay "child commit", not "parent commit" - err = g.RebaseOnto(trunk, parentTip, "child") + err = g.RebaseOnto(trunk, parentTip, "child", false) if err != nil { t.Fatalf("RebaseOnto failed: %v", err) } @@ -791,3 +791,116 @@ func TestRemoteBranchExists(t *testing.T) { t.Error("RemoteBranchExists(nonexistent) = true, want false") } } + +// setupLinearStack creates a linear chain main → A → B → C and advances main, +// returning the repo dir. +func setupLinearStack(t *testing.T) (dir string) { + t.Helper() + dir = t.TempDir() + + run := func(args ...string) string { + cmd := exec.Command("git", args...) + cmd.Dir = dir + out, err := cmd.Output() + if err != nil { + t.Fatalf("git %v failed: %v", args, err) + } + return strings.TrimSpace(string(out)) + } + + run("init", "-b", "main") + run("config", "user.email", "test@test.com") + run("config", "user.name", "Test") + + os.WriteFile(filepath.Join(dir, "f"), []byte("m1"), 0644) + run("add", ".") + run("commit", "-m", "m1") + + run("checkout", "-b", "A") + os.WriteFile(filepath.Join(dir, "a"), []byte("a"), 0644) + run("add", ".") + run("commit", "-m", "a") + + run("checkout", "-b", "B") + os.WriteFile(filepath.Join(dir, "b"), []byte("b"), 0644) + run("add", ".") + run("commit", "-m", "b") + + run("checkout", "-b", "C") + os.WriteFile(filepath.Join(dir, "c"), []byte("c"), 0644) + run("add", ".") + run("commit", "-m", "c") + + // Advance main so there is something to rebase onto + run("checkout", "main") + os.WriteFile(filepath.Join(dir, "f"), []byte("m2"), 0644) + run("add", ".") + run("commit", "-m", "m2") + + return dir +} + +// TestRebaseUpdateRefsMovesBookmark verifies that Rebase with updateRefs=true +// moves untracked bookmark branches that point into the rebased range. +// Uses a plain rebase (not --onto) so the full chain A→B→C is replayed and +// both B's ref and the bookmark (which points to B) are updated. +func TestRebaseUpdateRefsMovesBookmark(t *testing.T) { + dir := setupLinearStack(t) + g := git.New(dir) + + bBefore, err := g.GetTip("B") + if err != nil { + t.Fatalf("GetTip(B) failed: %v", err) + } + + // Create an untracked bookmark pointing at B's current tip + if out, err := exec.Command("git", "-C", dir, "branch", "bookmark", "B").CombinedOutput(); err != nil { + t.Fatalf("git branch bookmark B failed: %v\n%s", err, out) + } + + // Checkout C and rebase the whole chain (A→B→C) onto new main with --update-refs. + // B's ref and bookmark should both move to the rebased equivalent of B's commit. + if out, err := exec.Command("git", "-C", dir, "checkout", "C").CombinedOutput(); err != nil { + t.Fatalf("git checkout C failed: %v\n%s", err, out) + } + if err := g.Rebase("main", true); err != nil { + t.Fatalf("Rebase failed: %v", err) + } + + bAfter, _ := g.GetTip("B") + bookmarkAfter, _ := g.GetTip("bookmark") + + if bAfter == bBefore { + t.Error("expected B to be updated by --update-refs, but it was unchanged") + } + if bookmarkAfter != bAfter { + t.Errorf("expected bookmark to track new B tip %s, got %s", bAfter, bookmarkAfter) + } +} + +// TestRebaseNoUpdateRefsPreservesBookmark verifies that Rebase with +// updateRefs=false preserves untracked bookmark branches. +func TestRebaseNoUpdateRefsPreservesBookmark(t *testing.T) { + dir := setupLinearStack(t) + g := git.New(dir) + + // Create an untracked bookmark pointing at B's current tip + if out, err := exec.Command("git", "-C", dir, "branch", "bookmark", "B").CombinedOutput(); err != nil { + t.Fatalf("git branch bookmark B failed: %v\n%s", err, out) + } + bookmarkBefore, _ := g.GetTip("bookmark") + + // Checkout C and rebase with --no-update-refs; bookmark should not move + if out, err := exec.Command("git", "-C", dir, "checkout", "C").CombinedOutput(); err != nil { + t.Fatalf("git checkout C failed: %v\n%s", err, out) + } + if err := g.Rebase("main", false); err != nil { + t.Fatalf("Rebase failed: %v", err) + } + + bookmarkAfter, _ := g.GetTip("bookmark") + + if bookmarkAfter != bookmarkBefore { + t.Errorf("expected bookmark to be preserved with --no-update-refs, but it moved from %s to %s", bookmarkBefore, bookmarkAfter) + } +} diff --git a/internal/state/state.go b/internal/state/state.go index 2caad10..944ebb3 100644 --- a/internal/state/state.go +++ b/internal/state/state.go @@ -42,6 +42,11 @@ type RestackState struct { // Persisted so that continue/abort can find the correct worktree // directory for branches that were being rebased in a linked worktree. Worktrees map[string]string `json:"worktrees,omitempty"` + // UpdateRefs is the resolved --update-refs setting from the original + // invocation, persisted so that `continue` rebases remaining branches + // with the same setting after a conflict. False (zero value) means + // --no-update-refs was in effect, which is safe for old state files. + UpdateRefs bool `json:"update_refs,omitempty"` } // Save persists restack state to .git/STACK_CASCADE_STATE.