Skip to content

Commit 2fa1894

Browse files
committed
Preserve rewrite checkout during stack selection
1 parent c74d5a2 commit 2fa1894

7 files changed

Lines changed: 203 additions & 5 deletions

File tree

‎cmd/rebase_test.go‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2491,6 +2491,97 @@ func setupWorktreeRebaseRepo(t *testing.T, conflict bool) worktreeRebaseRepo {
24912491
return worktreeRebaseRepo{repo, parentDir, childDir}
24922492
}
24932493

2494+
func setupSharedTrunkRebaseRepo(t *testing.T, conflict bool) worktreeRebaseRepo {
2495+
t.Helper()
2496+
repo := setupWorktreeRebaseRepo(t, conflict)
2497+
issue250Git(t, repo.childDir, "checkout", "--detach")
2498+
issue250Git(t, repo.dir, "branch", "independent", "main")
2499+
sf, err := stack.Load(repo.gitDir)
2500+
require.NoError(t, err)
2501+
sf.AddStack(stack.Stack{Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "independent"}}})
2502+
require.NoError(t, stack.Save(repo.gitDir, sf))
2503+
return repo
2504+
}
2505+
2506+
func TestRebase_SharedTrunkSelectionPreservesOriginAndRange(t *testing.T) {
2507+
for _, tc := range []struct {
2508+
name string
2509+
opts rebaseOptions
2510+
want []string
2511+
}{
2512+
{name: "whole stack", want: []string{"b1", "b2", "b3"}},
2513+
{name: "upstack from trunk", opts: rebaseOptions{upstack: true}, want: []string{"b1", "b2", "b3"}},
2514+
{name: "downstack from trunk", opts: rebaseOptions{downstack: true}, want: []string{"b1"}},
2515+
{name: "explicit upstack", opts: rebaseOptions{branch: "b2", upstack: true}, want: []string{"b2", "b3"}},
2516+
{name: "explicit downstack", opts: rebaseOptions{branch: "b2", downstack: true}, want: []string{"b1", "b2"}},
2517+
{name: "without trunk", opts: rebaseOptions{noTrunk: true}, want: []string{"b2", "b3"}},
2518+
} {
2519+
t.Run(tc.name, func(t *testing.T) {
2520+
dir := t.TempDir()
2521+
writeStackFileMulti(t, dir,
2522+
stack.Stack{Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "b1"}, {Branch: "b2"}, {Branch: "b3"}}},
2523+
stack.Stack{Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "independent"}}},
2524+
)
2525+
current := "main"
2526+
var rebased []string
2527+
mock := newRebaseMock(dir, current)
2528+
mock.CurrentBranchFn = func() (string, error) { return current, nil }
2529+
mock.BranchExistsFn = func(string) (bool, error) { return true, nil }
2530+
mock.CheckoutBranchFn = func(branch string) error { current = branch; return nil }
2531+
mock.RebaseFn = func(string, git.RebaseOpts) error { rebased = append(rebased, current); return nil }
2532+
mock.RebaseOntoFn = func(_, _, branch string, _ git.RebaseOpts) error {
2533+
rebased = append(rebased, branch)
2534+
return nil
2535+
}
2536+
mock.IsRerereEnabledFn = func() (bool, error) { return true, nil }
2537+
restore := git.SetOps(mock)
2538+
defer restore()
2539+
cfg := issue250TestConfig(t)
2540+
cfg.ForceInteractive = true
2541+
cfg.SelectFn = func(_, _ string, _ []string) (int, error) { return 0, nil }
2542+
tc.opts.remote = "origin"
2543+
2544+
require.NoError(t, runRebase(cfg, &tc.opts))
2545+
2546+
assert.Equal(t, tc.want, rebased)
2547+
assert.Equal(t, "main", current)
2548+
assert.NoFileExists(t, filepath.Join(dir, rebaseStateFile))
2549+
})
2550+
}
2551+
}
2552+
2553+
func TestRebase_SharedTrunkSelectionRecoveryPreservesOrigin(t *testing.T) {
2554+
for _, abort := range []bool{false, true} {
2555+
t.Run(fmt.Sprintf("abort=%t", abort), func(t *testing.T) {
2556+
repo := setupSharedTrunkRebaseRepo(t, true)
2557+
observerHead := issue250Git(t, repo.childDir, "rev-parse", "HEAD")
2558+
withIssue250Repo(t, repo.dir)
2559+
cfg := issue250TestConfig(t)
2560+
cfg.ForceInteractive = true
2561+
selections := 0
2562+
cfg.SelectFn = func(_, _ string, _ []string) (int, error) { selections++; return 0, nil }
2563+
cfg.ConfirmFn = func(string, bool) (bool, error) { return false, nil }
2564+
require.ErrorIs(t, runRebase(cfg, &rebaseOptions{remote: "origin"}), ErrConflict)
2565+
state, err := loadRebaseState(repo.gitDir)
2566+
require.NoError(t, err)
2567+
assert.Equal(t, "main", state.OriginalBranch)
2568+
if !abort {
2569+
issue250WriteFile(t, repo.dir, "base.txt", "resolved\n")
2570+
issue250Git(t, repo.dir, "add", "base.txt")
2571+
}
2572+
withIssue250Repo(t, repo.parentDir)
2573+
2574+
require.NoError(t, runRebase(cfg, &rebaseOptions{abort: abort, cont: !abort}))
2575+
2576+
assert.Equal(t, "main", issue250Git(t, repo.dir, "branch", "--show-current"))
2577+
assert.Equal(t, "parent", issue250Git(t, repo.parentDir, "branch", "--show-current"))
2578+
assert.Equal(t, observerHead, issue250Git(t, repo.childDir, "rev-parse", "HEAD"))
2579+
assert.Equal(t, 1, selections)
2580+
assert.NoFileExists(t, filepath.Join(repo.gitDir, rebaseStateFile))
2581+
})
2582+
}
2583+
}
2584+
24942585
func TestRebase_WorktreesPreserveCheckouts(t *testing.T) {
24952586
repo := setupWorktreeRebaseRepo(t, false)
24962587
issue250WriteFile(t, repo.dir, "unrelated.txt", "leave main alone\n")

‎cmd/sync_test.go‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2634,6 +2634,23 @@ func TestSync_WorktreesPublishesFromLinkedWorktree(t *testing.T) {
26342634
require.NoError(t, issue250GitMayFail(t, repo.dir, "merge-base", "--is-ancestor", "parent", "child"))
26352635
}
26362636

2637+
func TestSync_SharedTrunkSelectionRestoresOrigin(t *testing.T) {
2638+
repo := setupSharedTrunkRebaseRepo(t, false)
2639+
observerHead := issue250Git(t, repo.childDir, "rev-parse", "HEAD")
2640+
withIssue250Repo(t, repo.dir)
2641+
cfg := issue250TestConfig(t)
2642+
cfg.ForceInteractive = true
2643+
cfg.SelectFn = func(_, _ string, _ []string) (int, error) { return 0, nil }
2644+
cfg.ConfirmFn = func(string, bool) (bool, error) { return false, nil }
2645+
2646+
require.NoError(t, runSync(cfg, &syncOptions{remote: "origin"}))
2647+
2648+
assert.Equal(t, "main", issue250Git(t, repo.dir, "branch", "--show-current"))
2649+
assert.Equal(t, "parent", issue250Git(t, repo.parentDir, "branch", "--show-current"))
2650+
assert.Equal(t, observerHead, issue250Git(t, repo.childDir, "rev-parse", "HEAD"))
2651+
assert.Equal(t, issue250Git(t, repo.dir, "rev-parse", "child"), issue250Git(t, repo.dir, "rev-parse", "origin/child"))
2652+
}
2653+
26372654
func TestSync_WorktreesConflictRestoresWithoutPush(t *testing.T) {
26382655
repo := setupWorktreeRebaseRepo(t, true)
26392656
beforeParent := issue250Git(t, repo.dir, "rev-parse", "parent")

‎cmd/utils.go‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -407,9 +407,9 @@ func handleSaveError(cfg *config.Config, err error) error {
407407
// resolveStack finds the stack for the given branch, handling ambiguity when
408408
// a branch (typically a trunk) belongs to multiple stacks. If exactly one
409409
// stack matches, it is returned directly. If multiple stacks match, the user
410-
// is prompted to select one and the working tree is switched to the top branch
411-
// of the selected stack. Returns nil with no error if no stack contains the
412-
// branch.
410+
// is prompted to select one. Read-only and rewrite callers preserve their
411+
// checkout; other mutating callers switch to the selected stack's top branch.
412+
// Returns nil with no error if no stack contains the branch.
413413
func resolveStack(sf *stack.StackFile, branch string, cfg *config.Config) (*stack.Stack, error) {
414414
stacks := sf.FindAllStacksForBranch(branch)
415415

@@ -456,8 +456,9 @@ func resolveStack(sf *stack.StackFile, branch string, cfg *config.Config) (*stac
456456
if len(s.Branches) == 0 {
457457
return nil, fmt.Errorf("selected stack %q has no branches", s.DisplayChain())
458458
}
459-
// Read-only selection must remain usable while another operation is paused.
460-
if cfg.StackMutation == nil {
459+
// Selection must not change a rewrite's origin or bypass its preflight.
460+
// Read-only selection also remains available while an operation is paused.
461+
if cfg.StackMutation == nil || cfg.StackMutation.NoCheckoutOnSelect {
461462
return s, nil
462463
}
463464

‎cmd/utils_test.go‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,86 @@ func TestResolveStack_ReadOnlySelectionDoesNotCheckout(t *testing.T) {
7878
commandOutput(t, cfg, outR, errR)
7979
}
8080

81+
func TestResolveStack_RewriteSelectionPreservesCheckout(t *testing.T) {
82+
for _, kind := range []string{"rebase", "rebase-continue", "rebase-abort", "sync", "modify", "modify-continue", "modify-abort", "add", "checkout", "submit"} {
83+
t.Run(kind, func(t *testing.T) {
84+
dir := t.TempDir()
85+
writeStackFileMulti(t, dir,
86+
stack.Stack{Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "b1"}, {Branch: "b2"}}},
87+
stack.Stack{Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "independent"}}},
88+
)
89+
current := "main"
90+
var checkouts []string
91+
mock := newRebaseMock(dir, current)
92+
mock.CurrentBranchFn = func() (string, error) { return current, nil }
93+
mock.CheckoutBranchFn = func(branch string) error {
94+
checkouts = append(checkouts, branch)
95+
current = branch
96+
return nil
97+
}
98+
restore := git.SetOps(mock)
99+
defer restore()
100+
cfg := issue250TestConfig(t)
101+
cfg.ForceInteractive = true
102+
cfg.SelectFn = func(_, _ string, choices []string) (int, error) {
103+
require.Len(t, choices, 2)
104+
return 0, nil
105+
}
106+
release, err := beginStackMutation(cfg, kind)
107+
require.NoError(t, err)
108+
defer release()
109+
110+
result, err := loadStackOptional(cfg, "")
111+
112+
require.NoError(t, err)
113+
assert.Equal(t, []string{"b1", "b2"}, result.Stack.BranchNames())
114+
if kind == "add" || kind == "checkout" || kind == "submit" {
115+
assert.Equal(t, []string{"b2"}, checkouts, "intentional selection checkout must remain available")
116+
assert.Equal(t, "b2", result.CurrentBranch)
117+
} else {
118+
assert.Empty(t, checkouts, "rewrite selection must not move the origin before preflight")
119+
assert.Equal(t, "main", result.CurrentBranch)
120+
}
121+
assert.Equal(t, result.CurrentBranch, current)
122+
})
123+
}
124+
}
125+
126+
func TestStackSelection_RewritePreflightPreservesCheckout(t *testing.T) {
127+
for _, command := range []struct {
128+
name string
129+
run func(*config.Config) error
130+
}{
131+
{"rebase", func(cfg *config.Config) error { return runRebase(cfg, &rebaseOptions{remote: "origin"}) }},
132+
{"sync", func(cfg *config.Config) error { return runSync(cfg, &syncOptions{remote: "origin"}) }},
133+
{"modify", runModify},
134+
} {
135+
t.Run(command.name, func(t *testing.T) {
136+
repo := setupSharedTrunkRebaseRepo(t, false)
137+
issue250WriteFile(t, repo.parentDir, "unfinished.txt", "preserve this work\n")
138+
beforeRefs := issue250Git(t, repo.dir, "show-ref")
139+
beforeCatalog, err := os.ReadFile(filepath.Join(repo.gitDir, "gh-stack"))
140+
require.NoError(t, err)
141+
withIssue250Repo(t, repo.dir)
142+
cfg := issue250TestConfig(t)
143+
cfg.ForceInteractive = true
144+
cfg.SelectFn = func(_, _ string, _ []string) (int, error) { return 0, nil }
145+
cfg.ConfirmFn = func(string, bool) (bool, error) { return false, nil }
146+
147+
require.Error(t, command.run(cfg))
148+
149+
assert.Equal(t, "main", issue250Git(t, repo.dir, "branch", "--show-current"))
150+
assert.Equal(t, "parent", issue250Git(t, repo.parentDir, "branch", "--show-current"))
151+
assert.Equal(t, beforeRefs, issue250Git(t, repo.dir, "show-ref"))
152+
afterCatalog, err := os.ReadFile(filepath.Join(repo.gitDir, "gh-stack"))
153+
require.NoError(t, err)
154+
assert.Equal(t, beforeCatalog, afterCatalog)
155+
assert.FileExists(t, filepath.Join(repo.parentDir, "unfinished.txt"))
156+
assert.NoFileExists(t, filepath.Join(repo.gitDir, rebaseStateFile))
157+
})
158+
}
159+
}
160+
81161
func TestStackMutation_NestedAndReadOnly(t *testing.T) {
82162
common := t.TempDir()
83163
restore := git.SetOps(&git.MockOps{GitDirFn: func() (string, error) { return common, nil }})

‎cmd/worktree_utils.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,10 @@ func beginStackMutation(cfg *config.Config, kind string) (func(), error) {
7878
return nil, err
7979
}
8080
cfg.StackMutation = &config.StackMutationContext{CommonDir: commonDir, StateDir: stateDir}
81+
switch kind {
82+
case "rebase", "rebase-continue", "rebase-abort", "sync", "modify", "modify-continue", "modify-abort":
83+
cfg.StackMutation.NoCheckoutOnSelect = true
84+
}
8185
released := false
8286
return func() {
8387
if !released {

‎docs/src/content/docs/guides/workflows.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,8 @@ gh-stack does not install shell functions or change your shell's directory. Do n
6767

6868
`rebase` and `sync` automatically operate in each affected branch's clean owning worktree. Unoccupied branches are processed in the initiating worktree, whose original checkout is restored afterward. Dirty, busy, missing, or changed affected owners block mutation; unrelated worktrees are left alone. A clean trunk owner can be fast-forwarded, while an unsafe local trunk retains the fetched-remote fallback. gh-stack never auto-stashes, transfers ownership, or creates/removes worktrees.
6969

70+
When a trunk belongs to multiple stacks, selecting one for `rebase`, `sync`, or `modify` does not switch branches. Rebase ranges use the caller's original checkout unless an explicit branch is supplied.
71+
7072
Resolve and stage conflicts in the worktree named by the diagnostic. You can run `gh stack rebase --continue` or `--abort` from any linked worktree: the shared journal routes recovery to the recorded owners. `sync` still restores its cascade on conflicts rather than pushing partial results; completed fetches and earlier fast-forwards are outside that rollback boundary. Recovery retains state and reports any partial failure rather than discarding later edits or claiming a full restoration. Pruning skips branches still occupied in other worktrees.
7173

7274
Finish paused operations before changing gh-stack versions or preview stages. Origin-only journals are explicitly marked; a build that cannot interpret a journal's execution lifecycle must leave it intact. If recovery reports an incompatible lifecycle, use the matching build in the recorded origin to finish or abort it instead of editing or removing the journal.

‎internal/config/config.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,9 @@ type Config struct {
6767
type StackMutationContext struct {
6868
CommonDir string
6969
StateDir string
70+
71+
// NoCheckoutOnSelect preserves rewrite origins and range anchors.
72+
NoCheckoutOnSelect bool
7073
}
7174

7275
// New creates a new Config with terminal-aware output and color support.

0 commit comments

Comments
 (0)