From f9a47f90c73dce9014994fa789a79ee5a071d2cc Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Wed, 16 Sep 2026 11:33:01 -0400 Subject: [PATCH] Support modifying stacks across worktrees --- .github/copilot-instructions.md | 2 +- AGENTS.md | 4 +- README.md | 4 +- cmd/modify.go | 8 +- cmd/modify_test.go | 203 +++- .../docs/getting-started/quick-start.md | 2 +- docs/src/content/docs/guides/modify.md | 29 +- docs/src/content/docs/guides/workflows.md | 6 +- docs/src/content/docs/reference/cli.md | 10 +- internal/modify/actions.go | 267 +++++ internal/modify/apply.go | 654 ++++-------- internal/modify/apply_test.go | 972 +++++++++++++++++- internal/modify/plan.go | 255 +++++ internal/modify/preconditions.go | 115 ++- internal/modify/recovery.go | 406 +++++++- internal/modify/state.go | 9 + skills/gh-stack/SKILL.md | 7 +- skills/gh-stack/references/troubleshooting.md | 23 +- 18 files changed, 2395 insertions(+), 581 deletions(-) create mode 100644 internal/modify/actions.go create mode 100644 internal/modify/plan.go diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index a383a36..40d4a02 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -38,6 +38,6 @@ No Makefile, no code generation, no external linter config. Standard Go toolchai - Use `stackStateDir(cfg)` for application state and `beginStackMutation` before mutation snapshots; defer cleanup. The clone-wide operation lock is separate from short catalog saves. - Recovery must match stack identity, execute in the recorded worktree, and retain journals on partial failures. Native Git markers stay per-worktree. - Mutation locks coordinate gh-stack only, not Git commands/editors. Keep affected worktrees quiescent during rewrites. Pass the snapshot SHA (or prior `Context.Touched` SHA) to `Context.Start` before ref mutations; never claim an external commit as this operation's work during continuation. -- Core modify rejects distributed stack branches before TUI/apply; foreign trunk ownership alone is allowed. Do not enable distributed modify until its dependent layer is implemented. +- Distributed modify preflights action/cascade targets and executes in recorded owners; only the origin may switch for unoccupied branches. Preserve dropped/folded source worktrees and refs. Before native continuation, preflight other worktrees, not remaining branches in the intentionally busy pending worktree. For full architecture details, see [AGENTS.md](../AGENTS.md) in the repository root. diff --git a/AGENTS.md b/AGENTS.md index 0623ead..b63f3d6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -125,8 +125,8 @@ if errors.As(err, &exitErr) { ... } - **Migration:** Consolidate only nonconflicting legacy catalogs and preserve originals. Stop on conflicting definitions; finish legacy recovery in its original worktree before migrating. Do not mix old and new writers. - **Recovery:** gh-stack journals live in the common directory and record origin/owner identities, original refs, and progress. Native Git markers remain per-worktree. Continue/abort must use recorded scoped executors, match stack identity (not catalog array position), and retain state on any partial restore or save failure. - **External changes:** Mutation locks coordinate gh-stack, not arbitrary Git commands or editors. Keep affected worktrees quiescent during rewrites, except for requested conflict resolution while paused. Call `Context.Start(branch, expectedSHA)` before ref mutations: use the original snapshot SHA, or the last `Context.Touched` SHA for a branch already changed by this operation. Do not adopt a freshly read tip as this operation's baseline during continuation. -- **Separate Git directories:** Native topology may report the administration directory as the main worktree path for `--separate-git-dir` repositories. A known origin remains usable, but a foreign main-owner root may be undiscoverable. Never infer a working directory from an administration path, emit it as a successful navigation target, or add a private registry/config mutation to guess ownership. -- **Core modify boundary:** Plain modify permits unoccupied branches and branches owned by its origin worktree, but rejects distributed stack membership before the TUI/apply. Trunk ownership alone does not block it. Full distributed modify is a separate layer. +- **Separate Git directories:** Main-worktree invocation and existing absolute/relative `core.worktree` backlinks, including the main `config.worktree`, are supported. The discovery caveat is only linked invocation without a main-worktree backlink: fail actionably if that owner is required, but allow unaffected worktrees to proceed. Never infer a working directory from an administration path, emit it as a successful navigation target, or add a private registry/config mutation to guess ownership. +- **Distributed modify:** Preflight the staged actions and surviving cascade branches before mutation. Run renames and history rewrites in recorded owners, using the origin only for unoccupied branches. Persist execution progress, rename aliases, created refs, and last-written heads. Before native continuation, preflight other target worktrees; the pending worktree is intentionally busy and its later branches are checked after continuation. Never switch a foreign owner to another branch, reset external commits, delete a preserved source branch/worktree, or clear a partially restored journal. ## CI workflows (`.github/workflows/`) diff --git a/README.md b/README.md index 26e2090..b158c9e 100644 --- a/README.md +++ b/README.md @@ -76,9 +76,9 @@ Mutation locks coordinate **gh-stack processes only**, not arbitrary Git command Navigation to a branch checked out elsewhere reports its path and fails without switching. Add `--print-path` to `up`, `down`, `top`, `bottom`, `trunk`, or an explicit-target `checkout` to get the owning path instead. Unoccupied targets are checked out here before printing this worktree's path; successful stdout contains only the absolute path and a newline. See the [worktree workflow](docs/src/content/docs/guides/workflows.md#working-across-git-worktrees) for a shell wrapper that checks errors before changing directories. -**Temporary core limitation:** `modify` works inside a linked worktree only when all stack branches are unoccupied or checked out in that same worktree. Distributed modify is rejected before opening the TUI or applying changes. A trunk checked out elsewhere is allowed because modify only reads it. +`modify` supports stacks distributed across worktrees. It preflights the owners needed by the staged actions and cascade, renames in each branch's owner, and cherry-picks or rebases in the receiving owner's worktree. Unoccupied branches use the initiating worktree; other worktrees are never switched to different branches. Dropped/folded branches and their worktrees remain intact. If the origin's nearest surviving branch is owned elsewhere, modify keeps the preserved original branch checked out and reports the survivor's path. Trunk is only read. -With `git init --separate-git-dir`, Git may report the administration directory instead of the main working directory in its worktree list. Shared storage and operations from a known main or linked worktree still work, but discovering that main worktree's owner path from another checkout can be unavailable. Do not treat an administration-directory path as a checkout directory; use the actual main worktree when its location cannot be discovered. +Repositories created with `git init --separate-git-dir` support main-worktree invocation and main-owner discovery through an existing absolute or relative `core.worktree` backlink, including one stored in the main `config.worktree`. The discovery limitation is only linked-worktree invocation without a main-worktree backlink: an operation requiring that main owner fails with actionable guidance, while unaffected worktrees continue. An administration directory is never treated as a checkout destination. ## Commands diff --git a/cmd/modify.go b/cmd/modify.go index 6792953..b3c1975 100644 --- a/cmd/modify.go +++ b/cmd/modify.go @@ -35,6 +35,10 @@ Operations available: • Rename branches All changes are staged in the TUI and applied together when you press Ctrl+S. +Branches may be checked out in different worktrees. Changes run in their clean +owning worktrees; unoccupied branches use the initiating worktree. Foreign +worktrees are never switched to different branches. Changes are never autostashed, +and no worktrees are created or removed. If your changes affect branches with pull requests, run 'gh stack submit' afterward to push changes, update PRs, and recreate the stack on GitHub.`, Example: ` # Open the interactive TUI to restructure the stack @@ -168,11 +172,11 @@ func runModify(cfg *config.Config) error { if state == nil || state.Worktrees == nil { return fmt.Errorf("modify conflict has no recorded worktree; recovery state was retained") } - ops, err := state.Worktrees.OriginOps() + ops, path, err := modify.ConflictOps(state) if err != nil { return err } - printConflictDetailsAt(cfg, ops, state.Worktrees.Origin.Path, conflict.Branch, "gh stack modify --continue") + printConflictDetailsAt(cfg, ops, path, conflict.Branch, "gh stack modify --continue") cfg.Printf("") cfg.Printf("Or restore the stack to its pre-modify state with `%s`", diff --git a/cmd/modify_test.go b/cmd/modify_test.go index 9ceb2e2..fc1477a 100644 --- a/cmd/modify_test.go +++ b/cmd/modify_test.go @@ -9,6 +9,7 @@ import ( "testing" "time" + tea "github.com/charmbracelet/bubbletea" "github.com/github/gh-stack/internal/config" "github.com/github/gh-stack/internal/git" "github.com/github/gh-stack/internal/github" @@ -924,6 +925,12 @@ func TestCheckModifyPreconditions_Worktrees(t *testing.T) { }, nil }, } + mock.ForWorktreeFn = func(path string) git.Ops { + if worktree.SamePath(path, foreign) { + return &git.MockOps{RootDirFn: func() (string, error) { return foreign, nil }} + } + return mock + } restore := git.SetOps(mock) defer restore() cfg, _, errR := config.NewTestConfig() @@ -940,14 +947,9 @@ func TestCheckModifyPreconditions_Worktrees(t *testing.T) { cfg.Err.Close() output, readErr := io.ReadAll(errR) require.NoError(t, readErr) - if ownerBranch == "b2" { - require.Error(t, err) - assert.Contains(t, string(output), "distributed modify is not supported yet") - assert.Contains(t, string(output), foreign) - assert.Zero(t, prQueries.Load(), "distributed guard must run before PR refresh or TUI") - } else { - require.NoError(t, err) - } + require.NoError(t, err) + assert.NotContains(t, string(output), "distributed modify is not supported") + assert.Positive(t, prQueries.Load(), "action-specific owner checks happen after the TUI produces its plan") assert.False(t, modify.StateExists(dir)) }) } @@ -991,12 +993,18 @@ func TestRunModifyRecovery_UsesRecordedOrigin(t *testing.T) { require.NoError(t, modify.SaveState(common, state)) inProgress, continued, aborted := true, false, false sha := "original" + revParse := func(ref string) (string, error) { + if ref == "B" { + return "source", nil + } + return sha, nil + } originOps := &git.MockOps{ GitDirFn: func() (string, error) { return originDir, nil }, CommonDirFn: func() (string, error) { return common, nil }, RootDirFn: func() (string, error) { return origin, nil }, CurrentBranchFn: func() (string, error) { return "A", nil }, - RevParseFn: func(string) (string, error) { return sha, nil }, + RevParseFn: revParse, IsRebaseInProgressFn: func() bool { return inProgress && tc.conflictType == "rebase" }, RebaseContinueFn: func(git.RebaseOpts) error { require.Equal(t, "rebase", tc.conflictType) @@ -1026,7 +1034,7 @@ func TestRunModifyRecovery_UsesRecordedOrigin(t *testing.T) { CommonDirFn: func() (string, error) { return common, nil }, RootDirFn: func() (string, error) { return caller, nil }, CurrentBranchFn: func() (string, error) { return "observer", nil }, - RevParseFn: func(string) (string, error) { return sha, nil }, + RevParseFn: revParse, CheckoutBranchFn: func(string) error { callerSensitiveCalls++ return nil @@ -1099,6 +1107,49 @@ func TestModifyStateIOFailures(t *testing.T) { require.Error(t, modify.CheckStateGuard(dir)) } +func TestModifyApply_DoesNotReportAdministrationDirectoryAsOwner(t *testing.T) { + dir, origin := t.TempDir(), t.TempDir() + s := stack.Stack{Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "A"}, {Branch: "B"}}} + writeStackFile(t, dir, s) + mock := &git.MockOps{ + GitDirFn: func() (string, error) { return filepath.Join(dir, "worktrees", "origin"), nil }, + CommonDirFn: func() (string, error) { return dir, nil }, + RootDirFn: func() (string, error) { return origin, nil }, + CurrentBranchFn: func() (string, error) { return "B", nil }, + BranchExistsFn: func(string) bool { return true }, + RevParseFn: func(ref string) (string, error) { return "sha-" + ref, nil }, + WorktreesFn: func() ([]git.Worktree, error) { + return []git.Worktree{{Path: dir, Branch: "A"}, {Path: origin, Branch: "B"}}, nil + }, + } + mock.ForWorktreeFn = func(path string) git.Ops { + if worktree.SamePath(path, dir) { + return &git.MockOps{ + GitDirFn: func() (string, error) { return dir, nil }, + CommonDirFn: func() (string, error) { return dir, nil }, + RootDirFn: func() (string, error) { return "", assert.AnError }, + } + } + return mock + } + restore := git.SetOps(mock) + defer restore() + cfg, _, _ := config.NewTestConfig() + defer cfg.Out.Close() + defer cfg.Err.Close() + sf, err := stack.Load(dir) + require.NoError(t, err) + nodes := []modifyview.ModifyBranchNode{ + {BranchNode: stackview.BranchNode{Ref: s.Branches[1]}, OriginalPosition: 1}, + {BranchNode: stackview.BranchNode{Ref: s.Branches[0]}, OriginalPosition: 0}, + } + _, _, err = modify.ApplyPlan(cfg, dir, &sf.Stacks[0], sf, nodes, "B", func(*stack.Stack) {}) + require.Error(t, err) + assert.Contains(t, err.Error(), "working-tree root") + assert.NotContains(t, err.Error(), "checked out in worktree "+dir) + assert.False(t, modify.StateExists(dir)) +} + func TestRunModifyContinue_LegacyPrivateJournalKeepsOriginalCatalog(t *testing.T) { common, origin := t.TempDir(), t.TempDir() private := filepath.Join(common, "worktrees", "legacy") @@ -1172,3 +1223,135 @@ func TestRunModifyContinue_LegacyPrivateJournalKeepsOriginalCatalog(t *testing.T require.NoError(t, err) assert.Equal(t, []string{"A", "B", "C"}, privateCatalog.Stacks[0].BranchNames()) } + +func TestRunModifyContinue_UsesForeignPendingOwner(t *testing.T) { + common, origin, target, caller := t.TempDir(), t.TempDir(), t.TempDir(), t.TempDir() + refs := map[string]string{"main": "sha-main", "A": "sha-A", "C": "sha-C"} + s := stack.Stack{Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "A"}, {Branch: "C"}}} + writeStackFile(t, common, s) + metadata, err := json.Marshal(s) + require.NoError(t, err) + state := &modify.StateFile{ + SchemaVersion: 1, Phase: modify.PhaseConflict, ConflictType: "rebase", ConflictBranch: "A", + OriginalBranch: "C", RemainingBranches: []string{"C"}, OriginalRefs: map[string]string{"C": "sha-A"}, + Snapshot: modify.Snapshot{ + StackMetadata: metadata, + Branches: []modify.BranchSnapshot{{Name: "A", TipSHA: "sha-A"}, {Name: "C", TipSHA: "sha-C"}}, + }, + Worktrees: &worktree.Context{ + Origin: worktree.Location{Path: origin}, + Owners: map[string]*worktree.Location{"A": {Path: target}}, + Pending: "A", + PendingBefore: "sha-A", + }, + } + state.RecordStack(&s) + require.NoError(t, modify.SaveState(common, state)) + scoped := func(path, name string) *git.MockOps { + return &git.MockOps{ + RootDirFn: func() (string, error) { return path, nil }, + CommonDirFn: func() (string, error) { return common, nil }, + GitDirFn: func() (string, error) { return filepath.Join(common, "worktrees", name), nil }, + CurrentBranchFn: func() (string, error) { return name, nil }, + RevParseFn: func(ref string) (string, error) { return refs[ref], nil }, + IsAncestorFn: func(string, string) (bool, error) { return true, nil }, + MergeBaseFn: func(string, string) (string, error) { return "sha-A", nil }, + RebaseContinueFn: func(git.RebaseOpts) error { + t.Fatal("native continuation must run only in the pending owner's worktree") + return nil + }, + } + } + originOps, targetOps, callerOps := scoped(origin, "C"), scoped(target, "A"), scoped(caller, "observer") + inProgress, continued := true, false + targetOps.IsRebaseInProgressFn = func() bool { return inProgress } + targetOps.RebaseContinueFn = func(git.RebaseOpts) error { inProgress, continued = false, true; return nil } + callerOps.WorktreesFn = func() ([]git.Worktree, error) { + return []git.Worktree{{Path: origin, Branch: "C"}, {Path: target, Branch: "A"}, {Path: caller, Branch: "observer"}}, nil + } + callerOps.ForWorktreeFn = func(path string) git.Ops { + if worktree.SamePath(path, target) { + return targetOps + } + if worktree.SamePath(path, origin) { + return originOps + } + return callerOps + } + restore := git.SetOps(callerOps) + defer restore() + nativeOps, path, err := modify.ConflictOps(state) + require.NoError(t, err) + assert.Same(t, targetOps, nativeOps) + assert.Equal(t, target, path) + cfg, _, _ := config.NewTestConfig() + defer cfg.Out.Close() + defer cfg.Err.Close() + require.NoError(t, runModifyContinue(cfg)) + assert.True(t, continued) + assert.False(t, modify.StateExists(common)) + assert.Nil(t, cfg.StackMutation) +} + +func TestModifyTUI_RejectsMixedReorderFold(t *testing.T) { + for _, scenario := range []struct { + name string + keys []rune + order []string + kind modifyview.ActionType + }{ + {"move B below A then fold up", []rune{'J', 'u'}, []string{"C", "A", "B"}, modifyview.ActionMove}, + {"fold B up then move", []rune{'u', 'J'}, []string{"C", "B", "A"}, modifyview.ActionFoldUp}, + } { + t.Run(scenario.name, func(t *testing.T) { + nodes := []modifyview.ModifyBranchNode{ + {BranchNode: stackview.BranchNode{Ref: stack.BranchRef{Branch: "C"}}, OriginalPosition: 0}, + {BranchNode: stackview.BranchNode{Ref: stack.BranchRef{Branch: "B"}, IsCurrent: true}, OriginalPosition: 1}, + {BranchNode: stackview.BranchNode{Ref: stack.BranchRef{Branch: "A"}}, OriginalPosition: 2}, + } + model := modifyview.New(nodes, stack.BranchRef{Branch: "main"}, "test") + for _, key := range scenario.keys { + updated, _ := model.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{key}}) + var ok bool + model, ok = updated.(modifyview.Model) + require.True(t, ok) + } + var order []string + for _, node := range model.Nodes() { + order = append(order, node.Ref.Branch) + if scenario.kind == modifyview.ActionMove { + assert.Nil(t, node.PendingAction, "fold must be rejected after reordering") + assert.False(t, node.Removed) + } + } + + assert.Equal(t, scenario.order, order) + require.Len(t, model.StagedActions(), 1, "only the first operation may be staged") + assert.Equal(t, scenario.kind, model.StagedActions()[0].Type) + }) + } +} + +func TestModifyTUI_DropThenFoldUpSkipsDroppedNeighbor(t *testing.T) { + nodes := []modifyview.ModifyBranchNode{ + {BranchNode: stackview.BranchNode{Ref: stack.BranchRef{Branch: "C"}}, OriginalPosition: 0}, + {BranchNode: stackview.BranchNode{Ref: stack.BranchRef{Branch: "B"}, IsCurrent: true}, OriginalPosition: 1}, + {BranchNode: stackview.BranchNode{Ref: stack.BranchRef{Branch: "A"}}, OriginalPosition: 2}, + } + model := modifyview.New(nodes, stack.BranchRef{Branch: "main"}, "test") + for _, key := range []rune{'x', 'j', 'u'} { + updated, _ := model.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{key}}) + var ok bool + model, ok = updated.(modifyview.Model) + require.True(t, ok) + } + actions := model.StagedActions() + require.Len(t, actions, 2) + assert.Equal(t, modifyview.ActionDrop, actions[0].Type) + assert.Equal(t, "B", actions[0].BranchName) + assert.Equal(t, modifyview.ActionFoldUp, actions[1].Type) + assert.Equal(t, "A", actions[1].BranchName) + assert.Equal(t, "C", actions[1].FoldTarget) + assert.True(t, model.Nodes()[1].Removed) + assert.True(t, model.Nodes()[2].Removed) +} diff --git a/docs/src/content/docs/getting-started/quick-start.md b/docs/src/content/docs/getting-started/quick-start.md index 9c7af72..6ab4593 100644 --- a/docs/src/content/docs/getting-started/quick-start.md +++ b/docs/src/content/docs/getting-started/quick-start.md @@ -99,7 +99,7 @@ Linked worktrees share the same local stack catalog. You can adopt branches alre `rebase` and `sync` automatically update affected clean owners. gh-stack does not auto-stash or manage worktree creation/removal. To navigate across worktrees, use `--print-path` and a shell wrapper that checks the command's exit status before `cd`; see [Working across Git worktrees](/gh-stack/guides/workflows/#working-across-git-worktrees). -For this core release, `modify` supports a stack within one worktree but temporarily rejects stack branches checked out in other worktrees. +`modify` can restructure a stack distributed across worktrees. It uses each affected branch's clean owner without switching other worktrees, and preserves the underlying branches/worktrees when dropping or folding layers. Conflict messages identify where to resolve and stage; `modify --continue` and `--abort` can be invoked from any linked worktree. ## What's Next? diff --git a/docs/src/content/docs/guides/modify.md b/docs/src/content/docs/guides/modify.md index b5e6e6d..317bad3 100644 --- a/docs/src/content/docs/guides/modify.md +++ b/docs/src/content/docs/guides/modify.md @@ -25,11 +25,11 @@ Before running `modify`, ensure: - No PR in the stack is queued for merge - Commit history is linear (run `gh stack rebase` first if needed) - Git 2.36 or later -- Every stack branch is unoccupied or checked out in the invoking worktree +- Worktrees needed by the staged actions and surviving cascade are clean and have no other Git operation in progress -Linked worktrees are supported, but **distributed modify is temporarily rejected** before the TUI opens and rechecked before applying. A trunk checked out elsewhere does not block modify: trunk is only read. No worktrees are created, removed, detached, or automatically stashed. +Branches may be distributed across worktrees. Before applying, modify checks the owners needed by the selected actions and the surviving cascade. It checks again immediately before a mutation; it never auto-stashes. Unrelated worktrees, merged branches, and dropped/folded sources that are only read are left untouched. Trunk is only read, so its ownership alone does not block modify. -This also applies when using a separate Git administration directory: modify can use its known main or linked origin, but discovering another main worktree's location may be unavailable. See [Separate Git administration directories](/gh-stack/guides/workflows/#separate-git-administration-directories). +Separate Git administration directories support a known main or linked modify origin. Main-owner discovery also supports existing absolute/relative `core.worktree` backlinks, including the main `config.worktree`. The discovery caveat is only linked invocation without a main-worktree backlink; unaffected worktrees remain usable. See [Separate Git administration directories](/gh-stack/guides/workflows/#separate-git-administration-directories). ## Opening the TUI @@ -43,27 +43,27 @@ The TUI shows your stack as a vertical list of branches with PR information, com ### Drop (`x`) -Removes a branch and its commits from the stack. The local branch and any associated PR are preserved. Upstream branches are rebased to exclude the dropped branch's unique commits. +Removes a branch and its commits from the stack. The local branch, its worktree, and any associated PR are preserved. Branches above it are rebased in their owning worktrees to exclude the dropped branch's unique commits. ### Fold down (`d`) -Absorbs the selected branch's commits into the branch below it (toward trunk) via cherry-pick. The folded branch is removed from the stack. +Absorbs the selected branch's commits into the surviving branch below it (toward trunk) via cherry-pick in that receiver's worktree. The folded branch is removed from the stack, but its underlying ref and worktree remain intact. ### Fold up (`u`) -Absorbs the selected branch's commits into the branch above it (away from trunk). Since the branch above already contains the folded branch's commits in its history, this is handled by adjusting what is considered the first unique commit for the branch. The folded branch is removed from the stack. +Absorbs the selected branch's commits into the surviving branch above it (away from trunk). Since that branch already contains the source's commits, modify adjusts its original-parent cutoff so its rebase includes both layers. The source is removed from stack membership only; its ref and worktree are preserved. ### Insert below / above (`i` / `I`) -Inserts a new empty branch into the stack at the cursor position. Lowercase `i` inserts below the cursor (toward trunk); uppercase `I` inserts above the cursor (away from trunk). An inline prompt appears to enter the new branch name. The branch is created at apply time, pointing at the parent branch's tip. +Inserts a new empty branch into the stack at the cursor position. Lowercase `i` inserts below the cursor (toward trunk); uppercase `I` inserts above the cursor (away from trunk). An inline prompt appears to enter the new branch name. The branch ref is created at apply time, pointing at its parent's tip; no worktree is created. ### Rename (`r`) -Opens an inline prompt to enter a new name for the branch. The branch is renamed locally and in the stack metadata. On the next `submit`, the new branch name is pushed to GitHub. +Opens an inline prompt to enter a new name for the branch. The branch is renamed in its owning worktree and in stack metadata, without moving the worktree directory or switching it to unrelated history. On the next `submit`, the new branch name is pushed to GitHub. ### Reorder (`Shift+↓`/`Shift+↑`) -Moves the selected branch down (toward trunk) or up (away from trunk) in the stack. A cascading rebase adjusts all affected branches. Note: reordering and structural changes (drop/fold/insert/rename) cannot be mixed in the same session. +Moves the selected branch down (toward trunk) or up (away from trunk) in the stack. A cascading rebase adjusts branches in their existing owners; unoccupied branches are processed in the initiating worktree. Note: reordering and structural changes (drop/fold/insert/rename) cannot be mixed in the same session. ### Undo (`z`) @@ -73,6 +73,8 @@ Reverses the most recent staged action. You can undo multiple times to step back Press `Ctrl+S` to apply all staged changes. Nothing is modified until you save. The apply phase renames branches, inserts new branches, folds/drops branches, and runs a cascading rebase to create a linear commit history with the desired stack state. +Other worktrees retain their branch choices throughout the operation. The initiating worktree returns to its original branch, using the new name if renamed. When that layer was dropped or folded, modify chooses the nearest surviving branch only if it is available here. If it is checked out elsewhere, the initiating worktree keeps the preserved original branch and reports the survivor's owning path instead. No worktrees are created, removed, or detached. + ### Handling conflicts If a rebase conflict occurs during the apply phase, you have two options: @@ -80,9 +82,9 @@ If a rebase conflict occurs during the apply phase, you have two options: 1. **Resolve and continue**: Fix the conflicts in your editor, stage with `git add`, then run `gh stack modify --continue` (you may need to do this multiple times) 2. **Abort**: Run `gh stack modify --abort` to abort the operation and restore the stack to the pre-modify state -If a second conflict occurs after continuing, the same options are available. +If a second conflict occurs after continuing, the same options are available. A fold-down cherry-pick can be followed by a rebase conflict in a different worktree; follow the newly reported path each time. Remaining structural actions are checkpointed and resumed, not skipped or repeated. -The conflict message identifies the originating worktree. Edit and stage the files **there**. You can invoke `--continue` or `--abort` from any linked worktree; Git operations still execute in the recorded origin without changing the invoking worktree's checkout. +The conflict message identifies the **worktree with the active Git operation**, which may differ from the origin or the fold source. Edit and stage files there. You can invoke `--continue` or `--abort` from any linked worktree; native operations use their recorded owners rather than the caller's checkout. Other target worktrees are checked before continuing; remaining branches in the intentionally busy conflict worktree are checked after the native operation finishes. ## After modifying @@ -102,9 +104,9 @@ If you want to discard all changes and restore the stack to its pre-modify state gh stack modify --abort ``` -This also works if `modify` was interrupted (e.g., terminal crash). The shared `/gh-stack-modify-state` journal records the origin, original checkout, stack identity, and pre-modify snapshot before mutations. Git's native rebase/cherry-pick state stays in the origin's own Git directory. +This also works if `modify` was interrupted (e.g., terminal crash). The shared `/gh-stack-modify-state` journal records the origin, participating owners, original checkout, stack identity, action progress, and expected refs before/after mutations. Native rebase/cherry-pick state remains in the worktree running that Git operation. -Recovery restores changes made by this operation and the original checkout. If the owner is missing, refs were changed externally, or a restore/save fails, recovery stops and retains its journal instead of reporting success. Address the reported problem and retry `--abort`; do not delete the journal to bypass recovery. After a successful modify has reached pending-submit, `--abort` does not undo it and instead directs you to `submit`. +Recovery reverses renames in their owners, restores only operation-touched refs, removes only branch refs proven to have been created by this modify, and restores the origin's original checkout. It never deletes a worktree or resets a preserved drop/fold source just because that source is in the snapshot. If an owner is missing, refs or ownership changed externally, or a restore/save fails, recovery stops and retains its journal instead of reporting success. Address the reported problem and retry `--abort`; do not delete the journal to bypass recovery. After a successful modify has reached pending-submit, `--abort` does not undo it and instead directs you to `submit`. Clone-wide mutation serialization prevents another gh-stack mutation while modify is applying or paused; read-only views remain available. Pending-submit state is consumed only when submitting its matching stack, never an unrelated stack. @@ -119,4 +121,3 @@ Legacy journals must be continued or aborted in their original worktree before c - Cannot move branches between different stacks - Requires an interactive terminal - Reordering and structural changes (drop/fold/insert/rename) cannot be mixed in the same session -- Distributed rename/fold/reorder support is deferred to the second layer; all member branches must currently be available in one worktree diff --git a/docs/src/content/docs/guides/workflows.md b/docs/src/content/docs/guides/workflows.md index 0b19356..94ea839 100644 --- a/docs/src/content/docs/guides/workflows.md +++ b/docs/src/content/docs/guides/workflows.md @@ -17,9 +17,9 @@ On upgrade, gh-stack automatically consolidates nonconflicting legacy catalogs, ### Separate Git administration directories -Repositories created with `git init --separate-git-dir` keep the administration directory outside the main working directory. Their shared catalog and operations in an explicitly known main or linked worktree remain supported. +Repositories created with `git init --separate-git-dir` keep the administration directory outside the main working directory. Shared storage and main-worktree invocation are supported. Linked worktrees can discover the main owner from an existing absolute or relative `core.worktree` backlink, including one stored in the main worktree's `config.worktree`. -There is a discovery limitation: Git's worktree list can report that administration directory as the main path, without a reverse pointer to the real main working directory. Automatic discovery of the main owner from a linked checkout may therefore be unavailable. If an operation needs that owner's working files, start from the actual main worktree instead. Do not `cd` into an administration directory or infer the checkout from its parent directory. gh-stack does not add a private worktree registry or change Git configuration to repair discovery. +The remaining discovery limitation is **linked invocation without a main-worktree backlink**. In that case, Git may report the administration directory rather than the main working directory. An operation requiring the unresolved main owner fails with actionable guidance to run from the main worktree or supply the backlink; operations on unaffected worktrees continue. Do not `cd` into an administration directory or infer the checkout from its parent directory. gh-stack reads existing backlinks but does not add a private worktree registry or change Git configuration to repair discovery. ### Adopt existing branches @@ -71,7 +71,7 @@ Resolve and stage conflicts in the worktree named by the diagnostic. You can run gh-stack serializes mutations across the clone, including independent stacks. Read-only views remain available. A paused rebase or modify journal blocks new gh-stack mutations until recovery. These locks coordinate **gh-stack only**, not arbitrary Git commands, editors, or other tools. Keep affected worktrees quiescent while history is being rewritten. During a pause, make only the requested conflict-resolution edits and staging in the reported worktree; avoid unrelated commits or checkout changes on participating branches. -**Core modify limitation:** `modify` works inside a linked worktree only when every stack branch is unoccupied or checked out there. Distributed modify is temporarily rejected before the TUI or apply changes. Trunk ownership alone is allowed. Its `--continue` and `--abort` use the recorded origin even when invoked elsewhere; see [Restructuring stacks](/gh-stack/guides/modify/). +**Distributed modify:** `modify` supports renaming, inserting, dropping, folding, and reordering branches across worktrees. It preflights affected owners, runs each rename/rewrite in the appropriate worktree, and uses the origin for unoccupied branches. Other worktrees keep their branch choices; dropping/folding a layer preserves its branch and worktree. If the nearest surviving branch is owned elsewhere, the origin keeps its preserved branch and reports the survivor's path. Conflicts are resolved in the reported owner, while `--continue` and `--abort` can be invoked from any linked worktree. See [Restructuring stacks](/gh-stack/guides/modify/). ## Standard Workflow diff --git a/docs/src/content/docs/reference/cli.md b/docs/src/content/docs/reference/cli.md index 2d8ea59..e31b530 100644 --- a/docs/src/content/docs/reference/cli.md +++ b/docs/src/content/docs/reference/cli.md @@ -21,7 +21,7 @@ All linked worktrees share `/gh-stack` and gh-stack recovery journal `rebase` and `sync` automatically update affected clean owners; dirty, busy, missing, or changed owners block unsafe updates. Neither command auto-stashes or manages worktree creation/removal. Shared-journal `--continue` and `--abort` use the recorded worktree, not the caller's checkout. Partial recovery failures retain state. See [Working across Git worktrees](/gh-stack/guides/workflows/#working-across-git-worktrees) for adoption, migration, and recovery details. -For repositories created with `git init --separate-git-dir`, operations from a known worktree remain supported, but automatic discovery of the main working directory from another checkout may be unavailable. Git's reported main path can be the administration directory rather than a usable checkout; do not use it as a working-directory navigation target. +For repositories created with `git init --separate-git-dir`, main-worktree invocation and existing absolute/relative `core.worktree` backlinks are supported, including settings in the main `config.worktree`. The discovery limitation is only linked invocation without a main-worktree backlink. If the operation requires that main owner, it fails with actionable guidance; unaffected worktrees continue. Administration directories are never used as checkout destinations. --- @@ -204,9 +204,9 @@ The command checks these conditions before opening the TUI: 3. No rebase in progress 4. No PR in the stack is queued for merge 5. Commit history must be linear (no merge commits, no diverged branches) -6. All stack branches must be unoccupied or checked out in the invoking worktree +6. Before applying, worktrees needed by the staged actions and surviving cascade must be clean and free of other Git operations -**Core limitation:** distributed modify is temporarily rejected before the TUI and rechecked before applying. A trunk checked out elsewhere is allowed because modify only reads it. +Stack branches may be distributed across worktrees. Renames run in the branch's owner; fold-down cherry-picks run in the receiver's owner; cascades rebase each branch in its owner. Unoccupied branches use the initiating worktree, and inserted branches are created as refs without new worktrees. Dropped/folded branches and their worktrees are preserved. Trunk is only read, and unrelated or untouched source worktrees do not need to be clean. **Operations:** @@ -230,7 +230,9 @@ If a rebase conflict occurs, you can: - Resolve conflicts, stage files, and run `gh stack modify --continue` - Or run `gh stack modify --abort` to abort the operation and restore the stack to the pre-modify state -Resolve and stage in the worktree named by the conflict message. Both recovery flags may be invoked from another linked worktree, but execute in the recorded origin and leave the caller's checkout alone. Failed restore or journal/catalog saves retain recovery state. Pending-submit state is consumed only for the matching stack. +Resolve and stage in the worktree named by the conflict message, which may be a foreign fold receiver or rebase owner. Both recovery flags may be invoked from any linked worktree; they use recorded owners rather than the caller's checkout. Continuation resumes remaining structural actions as well as rebases. Abort reverses renames in their owners, restores only operation-touched refs, and deletes only refs proven to have been created by this modify. Failed restore or journal/catalog saves retain recovery state. Pending-submit state is consumed only for the matching stack. + +Other worktrees are never switched to different branches. The origin returns to its original branch (including its new name after a rename), or the nearest surviving branch if available. If that survivor is owned elsewhere, the origin keeps the preserved original branch and reports the survivor's path instead. **After modifying:** diff --git a/internal/modify/actions.go b/internal/modify/actions.go new file mode 100644 index 0000000..7573d7d --- /dev/null +++ b/internal/modify/actions.go @@ -0,0 +1,267 @@ +package modify + +import ( + "errors" + "fmt" + "slices" + + "github.com/github/gh-stack/internal/config" + "github.com/github/gh-stack/internal/git" + "github.com/github/gh-stack/internal/stack" + "github.com/github/gh-stack/internal/tui/modifyview" +) + +func affectsBranch(s *stack.Stack, name string) bool { + for _, branch := range s.Branches { + if !branch.IsMerged() && branch.PullRequest != nil && + (branch.Branch == name || s.ActiveBaseBranch(branch.Branch) == name) { + return true + } + } + return false +} + +func runActions(cfg *config.Config, dir string, state *StateFile, s *stack.Stack, sf *stack.StackFile, result *modifyview.ApplyResult) (*modifyview.ConflictInfo, error) { + if state.NextAction < 0 || state.NextAction > len(state.Execution) { + return nil, fmt.Errorf("invalid modify action progress; recovery state was retained") + } + for state.NextAction < len(state.Execution) { + action := state.Execution[state.NextAction] + switch action.Type { + case "rename": + if err := applyRename(cfg, dir, state, s, sf, action, result); err != nil { + return nil, err + } + case "insert_above", "insert_below": + if err := applyInsert(cfg, dir, state, s, sf, action, result); err != nil { + return nil, err + } + case "fold_down", "fold_up": + conflict, err := applyFold(cfg, dir, state, s, sf, action) + if err != nil { + return conflict, err + } + case "fold_rebase": + if conflict, err := normalizeFoldReceiver(dir, state, s, sf, action); err != nil { + return conflict, err + } + cfg.Successf("Rebased %s to exclude dropped branch %s", action.Branch, action.Target) + case "drop": + index := s.IndexOf(action.Branch) + if index < 0 || s.Branches[index].IsMerged() { + return nil, fmt.Errorf("cannot drop %s from the recorded stack", action.Branch) + } + state.AffectsPRs = state.AffectsPRs || affectsBranch(s, action.Branch) + if pr := s.Branches[index].PullRequest; pr != nil { + result.DroppedPRs = append(result.DroppedPRs, modifyview.DroppedPR{Branch: action.Branch, PRNumber: pr.Number}) + } + s.Branches = slices.Delete(s.Branches, index, index+1) + cfg.Successf("Dropped %s from stack", action.Branch) + default: + return nil, fmt.Errorf("unknown modify execution action %q", action.Type) + } + state.PendingAction = nil + state.PendingHead = "" + state.NextAction++ + state.ConflictBranch, state.ConflictType = "", "actions" + if err := saveProgress(dir, state, s, sf); err != nil { + return nil, err + } + } + if err := applyOrder(s, state.DesiredOrder); err != nil { + return nil, err + } + state.ConflictBranch, state.ConflictType = "", "cascade" + state.RemainingBranches = s.BranchNames() + return nil, saveProgress(dir, state, s, sf) +} + +func normalizeFoldReceiver(dir string, state *StateFile, s *stack.Stack, sf *stack.StackFile, action Action) (*modifyview.ConflictInfo, error) { + index := s.IndexOf(action.Branch) + if index < 0 || s.Branches[index].IsSkipped() { + return nil, fmt.Errorf("cannot normalize fold receiver %s", action.Branch) + } + cutoff := originalTips(state)[action.Target] + base := state.OriginalRefs[action.Target] + if cutoff == "" || base == "" { + return nil, fmt.Errorf("no original range recorded for dropped branch %s", action.Target) + } + if err := checkExpectedRef(state, action.Branch); err != nil { + return nil, err + } + ops, err := branchOps(state.Worktrees, action.Branch) + if err != nil { + return nil, err + } + ancestor, err := ops.IsAncestor(cutoff, action.Branch) + if err != nil { + return nil, err + } + if !ancestor { + return nil, fmt.Errorf("dropped range %s is no longer identifiable in %s; recovery state was retained", action.Target, action.Branch) + } + state.PendingAction = &action + state.AffectsPRs = state.AffectsPRs || s.Branches[index].PullRequest != nil + return rebaseInOwner(dir, state, s, sf, action.Branch, base, cutoff) +} + +func applyRename(cfg *config.Config, dir string, state *StateFile, s *stack.Stack, sf *stack.StackFile, action Action, result *modifyview.ApplyResult) error { + index := s.IndexOf(action.Branch) + if index < 0 || s.Branches[index].IsMerged() { + return fmt.Errorf("cannot rename %s in the recorded stack", action.Branch) + } + ops, err := prepareMutation(state, action.Branch) + if err != nil { + return err + } + if ops.BranchExists(action.NewName) { + return fmt.Errorf("cannot rename %s to %s: branch already exists", action.Branch, action.NewName) + } + state.PendingAction = &action + ops, err = startRefMutation(dir, state, action.Branch) + if err != nil { + return err + } + if err := ops.RenameBranch(action.Branch, action.NewName); err != nil { + return errors.Join(fmt.Errorf("renaming %s to %s: %w", action.Branch, action.NewName, err), unwindState(cfg, dir, state, sf)) + } + state.Worktrees.Rename(action.Branch, action.NewName) + if state.RenamedBranches == nil { + state.RenamedBranches = make(map[string]string) + } + state.RenamedBranches[action.Branch] = action.NewName + if err := state.Worktrees.Record(action.NewName); err != nil { + return err + } + state.AffectsPRs = state.AffectsPRs || affectsBranch(s, action.Branch) + s.Branches[index].Branch = action.NewName + state.OriginalRefs[action.NewName] = state.OriginalRefs[action.Branch] + delete(state.OriginalRefs, action.Branch) + result.RenamedBranches = append(result.RenamedBranches, modifyview.RenamedBranch{OldName: action.Branch, NewName: action.NewName}) + cfg.Successf("Renamed %s → %s", action.Branch, action.NewName) + return nil +} + +func applyInsert(cfg *config.Config, dir string, state *StateFile, s *stack.Stack, sf *stack.StackFile, action Action, result *modifyview.ApplyResult) error { + if action.NewPosition < 0 || action.NewPosition > len(s.Branches) || !s.Contains(action.Target) { + return fmt.Errorf("invalid insertion position for %s", action.NewName) + } + ops, err := prepareMutation(state, action.NewName) + if err != nil { + return err + } + if ops.BranchExists(action.NewName) { + return fmt.Errorf("cannot insert %s: branch already exists", action.NewName) + } + if err := checkExpectedRef(state, action.Target); err != nil { + return err + } + parentSHA, err := ops.RevParse(action.Target) + if err != nil { + return fmt.Errorf("resolving insert parent %s: %w", action.Target, err) + } + state.PendingAction = &action + if err := SaveState(dir, state); err != nil { + return err + } + if err := ops.CreateBranch(action.NewName, parentSHA); err != nil { + return errors.Join(fmt.Errorf("creating %s from %s: %w", action.NewName, action.Target, err), unwindState(cfg, dir, state, sf)) + } + if state.CreatedBranches == nil { + state.CreatedBranches = make(map[string]string) + } + state.CreatedBranches[action.NewName] = parentSHA + state.OriginalRefs[action.NewName] = parentSHA + if err := state.Worktrees.Record(action.NewName); err != nil { + return err + } + s.Branches = slices.Insert(s.Branches, action.NewPosition, stack.BranchRef{Branch: action.NewName}) + state.AffectsPRs = state.AffectsPRs || affectsBranch(s, action.NewName) + result.InsertedBranches = append(result.InsertedBranches, action.NewName) + cfg.Successf("Inserted %s after %s", action.NewName, action.Target) + return nil +} + +func applyFold(cfg *config.Config, dir string, state *StateFile, s *stack.Stack, sf *stack.StackFile, action Action) (*modifyview.ConflictInfo, error) { + index, target := s.IndexOf(action.Branch), s.IndexOf(action.Target) + if index < 0 || target < 0 || index == target || s.Branches[index].IsMerged() || s.Branches[target].IsMerged() { + return nil, fmt.Errorf("cannot fold %s into %s in the recorded stack", action.Branch, action.Target) + } + if err := checkExpectedRef(state, action.Branch); err != nil { + return nil, err + } + cutoff := state.OriginalRefs[action.Branch] + if cutoff == "" { + return nil, fmt.Errorf("no original parent cutoff recorded for %s", action.Branch) + } + state.AffectsPRs = state.AffectsPRs || affectsBranch(s, action.Branch) || s.Branches[target].PullRequest != nil + if action.Type == "fold_up" { + // Multiple sources can fold into one receiver. Keep the earliest + // cutoff so a later fold cannot exclude an earlier source's commits. + current := state.OriginalRefs[action.Target] + previousFold := false + for _, completed := range state.Execution[:state.NextAction] { + if completed.Type == "fold_up" && completed.Target == action.Target { + previousFold = true + break + } + } + useCutoff := !previousFold || current == "" + if !useCutoff { + var err error + useCutoff, err = git.IsAncestor(cutoff, current) + if err != nil { + return nil, fmt.Errorf("comparing fold cutoffs for %s: %w", action.Target, err) + } + } + if useCutoff { + state.OriginalRefs[action.Target] = cutoff + } + cfg.Successf("Folded %s into %s", action.Branch, action.Target) + } else { + commits, err := git.LogRange(cutoff, action.Branch) + if err != nil { + return nil, fmt.Errorf("reading commits to fold from %s: %w", action.Branch, err) + } + if len(commits) > 0 { + state.PendingAction = &action + state.ConflictBranch, state.ConflictType = action.Branch, "cherry_pick" + state.FoldBranch, state.FoldTarget = action.Branch, action.Target + state.RemainingBranches = append([]string{}, state.DesiredOrder...) + _, err := startRefMutation(dir, state, action.Target) + if err != nil { + return nil, err + } + ops, err := state.Worktrees.Prepare(action.Target) + if err != nil { + return nil, err + } + shas := make([]string, len(commits)) + for i, commit := range commits { + shas[len(commits)-1-i] = commit.SHA + } + if err := ops.CherryPick(shas); err != nil { + files, fileErr := ops.ConflictedFiles() + if fileErr == nil && len(files) == 0 && !ops.IsCherryPickInProgress() { + return nil, errors.Join(fmt.Errorf("cherry-picking %s into %s: %w", action.Branch, action.Target, err), unwindState(cfg, dir, state, sf)) + } + if saveErr := saveNativeConflict(dir, state, s, sf, ops); saveErr != nil { + return nil, errors.Join(err, saveErr) + } + if fileErr != nil { + return nil, errors.Join(err, fileErr) + } + return &modifyview.ConflictInfo{Branch: action.Branch, ConflictedFiles: files}, + fmt.Errorf("cherry-pick conflict folding %s into %s in %s", action.Branch, action.Target, state.Worktrees.Location(action.Target).Path) + } + if err := state.Worktrees.Record(action.Target); err != nil { + return nil, err + } + cfg.Successf("Folded %s into %s (%d commits)", action.Branch, action.Target, len(commits)) + } else { + cfg.Printf("No commits to fold from %s", action.Branch) + } + } + s.Branches = slices.Delete(s.Branches, s.IndexOf(action.Branch), s.IndexOf(action.Branch)+1) + return nil, nil +} diff --git a/internal/modify/apply.go b/internal/modify/apply.go index 7db4604..a91d8bf 100644 --- a/internal/modify/apply.go +++ b/internal/modify/apply.go @@ -122,6 +122,13 @@ func ApplyPlan( if err != nil { return nil, nil, err } + if err := CheckNoMergeQueuePRs(cfg, s); err != nil { + return nil, nil, err + } + execution, desiredOrder, err := compileActions(s, nodes) + if err != nil { + return nil, nil, err + } // Build the snapshot before any changes snapshot, err := BuildSnapshot(s) @@ -154,13 +161,13 @@ func ApplyPlan( Worktrees: ctx, RenamedBranches: make(map[string]string), CreatedBranches: make(map[string]string), + Execution: execution, + DesiredOrder: desiredOrder, } stateFile.RecordStack(s) result := &modifyview.ApplyResult{Success: true} - // Track whether any action affects a branch with a PR. - affectsPRs := false // Collect original refs for rebase --onto, including trunk branchNames := make([]string, 0, len(s.Branches)+1) branchNames = append(branchNames, s.Trunk.Branch) @@ -191,440 +198,29 @@ func ApplyPlan( } } stateFile.OriginalRefs = originalParentTips + stateFile.TrunkSHA = originalRefs[s.Trunk.Branch] + if err := preflightBranches(stateFile, remainingTargets(stateFile), ""); err != nil { + return nil, nil, err + } if err := SaveState(gitDir, stateFile); err != nil { return nil, nil, fmt.Errorf("saving modify state: %w", err) } - rollback := func(cause error) error { - return errors.Join(cause, unwindState(cfg, gitDir, stateFile, sf)) - } - - // Step 1: Renames - for i, n := range nodes { - if n.PendingAction != nil && n.PendingAction.Type == modifyview.ActionRename { - oldName := n.Ref.Branch - newName := n.PendingAction.NewName - ops, err := prepareMutation(stateFile) - if err != nil { - return nil, nil, err - } - if ops.BranchExists(newName) { - return nil, nil, rollback(fmt.Errorf("cannot rename %s to %s: branch already exists", oldName, newName)) - } - stateFile.PendingAction = &Action{Type: "rename", Branch: oldName, NewName: newName} - ops, err = startRefMutation(gitDir, stateFile, oldName) - if err != nil { - return nil, nil, err - } - if err := ops.RenameBranch(oldName, newName); err != nil { - return nil, nil, rollback(fmt.Errorf("renaming %s to %s: %w", oldName, newName, err)) - } - ctx.Rename(oldName, newName) - stateFile.RenamedBranches[oldName] = newName - stateFile.PendingAction = nil - if err := ctx.Record(newName); err != nil { - return nil, nil, err - } - - // Update in-memory state - idx := s.IndexOf(oldName) - if idx >= 0 { - // Update originalRefs key - if sha, ok := originalRefs[oldName]; ok { - originalRefs[newName] = sha - delete(originalRefs, oldName) - } - // Update originalParentTips key - if sha, ok := originalParentTips[oldName]; ok { - originalParentTips[newName] = sha - delete(originalParentTips, oldName) - } - s.Branches[idx].Branch = newName - } - // Update the node's ref for later steps - nodes[i].Ref.Branch = newName - - result.RenamedBranches = append(result.RenamedBranches, modifyview.RenamedBranch{ - OldName: oldName, - NewName: newName, - }) - if n.Ref.PullRequest != nil { - affectsPRs = true - } - stateFile.AffectsPRs = affectsPRs - if err := saveProgress(gitDir, stateFile, s, sf); err != nil { - return nil, nil, err - } - cfg.Successf("Renamed %s → %s", oldName, newName) - } - } - - // Step 2: Inserts — create new branches and add to stack metadata. - // Process in order so positions are stable. The node's position in the - // non-removed list determines the parent branch. - for _, n := range nodes { - if n.PendingAction == nil { - continue - } - if n.PendingAction.Type != modifyview.ActionInsertBelow && n.PendingAction.Type != modifyview.ActionInsertAbove { - continue - } - - newName := n.PendingAction.NewName - - // Determine the parent branch: find the position of this node among - // the non-removed, non-merged nodes in the apply-order list, then - // look at the branch just before it (toward trunk). - var parentBranch string - insertPos := -1 - - // Determine where in s.Branches the new branch should go. - // Walk the non-removed nodes to find the relative position. - nonRemovedPos := 0 - for _, other := range nodes { - if other.Removed || other.Ref.IsMerged() { - continue - } - if other.Ref.Branch == newName { - insertPos = nonRemovedPos - break - } - nonRemovedPos++ - } - - if insertPos <= 0 { - parentBranch = s.Trunk.Branch - } else { - // Find the branch at insertPos-1 among active branches - activeCount := 0 - for _, b := range s.Branches { - if b.IsMerged() { - continue - } - if activeCount == insertPos-1 { - parentBranch = b.Branch - break - } - activeCount++ - } - if parentBranch == "" { - parentBranch = s.Trunk.Branch - } - } - - ops, err := prepareMutation(stateFile) - if err != nil { - return nil, nil, err - } - if ops.BranchExists(newName) { - return nil, nil, rollback(fmt.Errorf("cannot insert %s: branch already exists", newName)) - } - parentSHA, err := ops.RevParse(parentBranch) - if err != nil { - return nil, nil, rollback(fmt.Errorf("resolving insert parent %s: %w", parentBranch, err)) - } - stateFile.PendingAction = &Action{Type: string(n.PendingAction.Type), Branch: n.Ref.Branch, NewName: newName} - if err := SaveState(gitDir, stateFile); err != nil { - return nil, nil, err - } - if err := ops.CreateBranch(newName, parentSHA); err != nil { - return nil, nil, rollback(fmt.Errorf("creating branch %s from %s: %w", newName, parentBranch, err)) - } - stateFile.CreatedBranches[newName] = parentSHA - stateFile.PendingAction = nil - if err := ctx.Record(newName); err != nil { - return nil, nil, err - } - - // Insert BranchRef into s.Branches at the correct position - newRef := stack.BranchRef{Branch: newName} - targetIdx := len(s.Branches) // default: append at end - if insertPos >= 0 { - // Map the active position back to s.Branches index - activeCount := 0 - for j, b := range s.Branches { - if b.IsMerged() { - continue - } - if activeCount == insertPos { - targetIdx = j - break - } - activeCount++ - } - } - s.Branches = append(s.Branches, stack.BranchRef{}) - copy(s.Branches[targetIdx+1:], s.Branches[targetIdx:]) - s.Branches[targetIdx] = newRef - - // Check if the branch above the insertion point has a PR — - // its base changes, so we need a submit - if targetIdx < len(s.Branches)-1 { - above := s.Branches[targetIdx+1] - if above.PullRequest != nil { - affectsPRs = true - } - } - - result.InsertedBranches = append(result.InsertedBranches, newName) - stateFile.AffectsPRs = affectsPRs - if err := saveProgress(gitDir, stateFile, s, sf); err != nil { - return nil, nil, err - } - cfg.Successf("Inserted %s after %s", newName, parentBranch) - } - - // Step 3: Folds — absorb one branch's commits into an adjacent branch. - // - // Fold-down: cherry-pick the folded branch's commits onto the target below. - // The target is below in the stack (closer to trunk), so it doesn't - // contain the folded branch's commits. Cherry-pick adds them. - // - // Fold-up: the target (above) already contains the folded branch's commits - // in its ancestry (it's stacked on top). Instead of cherry-picking, we - // adjust originalParentTips so the cascading rebase replays both the - // folded branch's commits AND the target's own commits when rebasing - // the target onto the folded branch's base. - for _, n := range nodes { - if n.PendingAction == nil { - continue - } - if n.PendingAction.Type != modifyview.ActionFoldDown && n.PendingAction.Type != modifyview.ActionFoldUp { - continue - } - - foldBranch := n.Ref.Branch - - // Determine target branch - var targetBranch string - foldIdx := s.IndexOf(foldBranch) - if foldIdx < 0 { - continue - } - - if n.PendingAction.Type == modifyview.ActionFoldDown { - // Target is the branch below (toward trunk) - if foldIdx == 0 { - continue - } - targetBranch = s.Branches[foldIdx-1].Branch - } else { - // Target is the branch above (away from trunk) - if foldIdx >= len(s.Branches)-1 { - continue - } - targetBranch = s.Branches[foldIdx+1].Branch - } - - baseBranch := s.ActiveBaseBranch(foldBranch) - - // Check if fold source or target has a PR - if n.Ref.PullRequest != nil { - affectsPRs = true - } - targetIdx := s.IndexOf(targetBranch) - if targetIdx >= 0 && s.Branches[targetIdx].PullRequest != nil { - affectsPRs = true - } - - if n.PendingAction.Type == modifyview.ActionFoldDown { - // Fold-down: cherry-pick the folded branch's commits onto the target. - commits, err := git.LogRange(baseBranch, foldBranch) - if err != nil { - return nil, nil, rollback(fmt.Errorf("reading commits to fold from %s: %w", foldBranch, err)) - } - if len(commits) == 0 { - cfg.Printf("No commits to fold from %s", foldBranch) - } else { - stateFile.ConflictBranch = foldBranch - stateFile.ConflictType = "cherry_pick" - stateFile.FoldBranch, stateFile.FoldTarget = foldBranch, targetBranch - stateFile.AffectsPRs = affectsPRs - ops, err := startRefMutation(gitDir, stateFile, targetBranch) - if err != nil { - return nil, nil, err - } - if err := ops.CheckoutBranch(targetBranch); err != nil { - return nil, nil, rollback(fmt.Errorf("checking out %s for fold: %w", targetBranch, err)) - } - - shas := make([]string, len(commits)) - for i, c := range commits { - shas[len(commits)-1-i] = c.SHA - } - - if err := ops.CherryPick(shas); err != nil { - conflict := &modifyview.ConflictInfo{Branch: foldBranch} - files, fileErr := ops.ConflictedFiles() - conflict.ConflictedFiles = files - - // Compute remaining branches for cascading rebase after cherry-pick resumes. - // Since folds happen before cascading rebase (Step 5), all non-merged, non-folded - // branches need rebasing. - remaining := make([]string, 0) - for _, br := range s.Branches { - if !br.IsMerged() && br.Branch != foldBranch { - remaining = append(remaining, br.Branch) - } - } - - // Save conflict state so --continue can resume the cherry-pick - stateFile.Phase = PhaseConflict - stateFile.ConflictBranch = foldBranch - stateFile.ConflictType = "cherry_pick" - stateFile.FoldBranch = foldBranch - stateFile.FoldTarget = targetBranch - stateFile.RemainingBranches = remaining - stateFile.AffectsPRs = affectsPRs - if saveErr := saveProgress(gitDir, stateFile, s, sf); saveErr != nil { - return nil, nil, errors.Join(err, saveErr) - } - if fileErr != nil { - return nil, nil, errors.Join(err, fmt.Errorf("reading conflicts in %s: %w", ctx.Origin.Path, fileErr)) - } - - return nil, conflict, fmt.Errorf("cherry-pick conflict folding %s into %s in %s", foldBranch, targetBranch, ctx.Origin.Path) - } - if err := ctx.Record(targetBranch); err != nil { - return nil, nil, err - } - - cfg.Successf("Folded %s into %s (%d commits)", foldBranch, targetBranch, len(commits)) - } - } else { - // Fold-up: the target (above) already has the folded branch's - // commits in its history. We adjust originalParentTips so the - // cascading rebase uses the folded branch's BASE as the cutoff, - // replaying both the folded branch's commits and the target's - // own commits onto the new parent. - originalParentTips[targetBranch] = originalParentTips[foldBranch] - cfg.Successf("Folded %s into %s", foldBranch, targetBranch) - } - - // Remove folded branch from stack metadata - foldIdx = s.IndexOf(foldBranch) // re-resolve in case earlier folds shifted indices - if foldIdx >= 0 && foldIdx < len(s.Branches) { - s.Branches = append(s.Branches[:foldIdx], s.Branches[foldIdx+1:]...) - } - stateFile.AffectsPRs = affectsPRs - if err := saveProgress(gitDir, stateFile, s, sf); err != nil { - return nil, nil, err - } - } - - // Step 4: Drops — remove from stack metadata - // Process in reverse order to preserve indices - for i := len(nodes) - 1; i >= 0; i-- { - n := nodes[i] - if n.PendingAction == nil || n.PendingAction.Type != modifyview.ActionDrop { - continue - } - - dropBranch := n.Ref.Branch - dropIdx := s.IndexOf(dropBranch) - if dropIdx < 0 { - continue - } - - if n.Ref.PullRequest != nil && n.Ref.PullRequest.Number > 0 { - result.DroppedPRs = append(result.DroppedPRs, modifyview.DroppedPR{ - Branch: dropBranch, - PRNumber: n.Ref.PullRequest.Number, - }) - affectsPRs = true - } - - s.Branches = append(s.Branches[:dropIdx], s.Branches[dropIdx+1:]...) - stateFile.AffectsPRs = affectsPRs - if err := saveProgress(gitDir, stateFile, s, sf); err != nil { - return nil, nil, err - } - cfg.Successf("Dropped %s from stack", dropBranch) - } - - // Step 5: Reorder — build the desired branch order from the remaining nodes - desiredOrder := make([]string, 0) - for _, n := range nodes { - if n.Removed { - continue - } - if n.PendingAction != nil && (n.PendingAction.Type == modifyview.ActionDrop || - n.PendingAction.Type == modifyview.ActionFoldDown || - n.PendingAction.Type == modifyview.ActionFoldUp) { - continue - } - if n.Ref.IsMerged() { - continue // Merged branches keep their position - } - desiredOrder = append(desiredOrder, n.Ref.Branch) - } - - // Check if reorder is needed by comparing with current stack order - currentOrder := make([]string, 0) - for _, b := range s.Branches { - if !b.IsMerged() { - currentOrder = append(currentOrder, b.Branch) - } - } - - needsReorder := false - if len(desiredOrder) == len(currentOrder) { - for i := range desiredOrder { - if desiredOrder[i] != currentOrder[i] { - needsReorder = true - break - } - } - } else { - needsReorder = true - } - - // Rebuild s.Branches in the desired order, preserving merged branches - // at their original positions. - if needsReorder { - // Build a queue of active branches in the desired order - desiredIdx := 0 - branchMap := make(map[string]stack.BranchRef) - for _, b := range s.Branches { - branchMap[b.Branch] = b - } - - newBranches := make([]stack.BranchRef, 0, len(s.Branches)) - for _, b := range s.Branches { - if b.IsMerged() { - // Merged branches stay at their original position - newBranches = append(newBranches, b) - } else { - // Substitute the next active branch from the desired order - if desiredIdx < len(desiredOrder) { - if sub, ok := branchMap[desiredOrder[desiredIdx]]; ok { - newBranches = append(newBranches, sub) - } - desiredIdx++ - } - } - } - - s.Branches = newBranches - if err := saveProgress(gitDir, stateFile, s, sf); err != nil { - return nil, nil, err - } + if conflict, err := runActions(cfg, gitDir, stateFile, s, sf, result); err != nil { + return nil, conflict, err } // Step 6: Replay each active branch's original commit range onto its new parent. - moved, conflict, err := rebaseRemaining(cfg, gitDir, stateFile, s, sf, s.BranchNames()) + moved, conflict, err := rebaseRemaining(cfg, gitDir, stateFile, s, sf, stateFile.RemainingBranches) if err != nil { return nil, conflict, err } result.MovedBranches = moved - // Check out the best branch — the original if it's still in the stack, - // otherwise the nearest surviving branch. - targetBranch := resolveCheckoutBranch(currentBranch, plan, snapshot, s) - if err := ctx.RestoreOrigin(targetBranch); err != nil { + if err := checkResultRefs(stateFile, s); err != nil { return nil, nil, err } - if targetBranch != currentBranch { - cfg.Printf("Switched to %s (original branch %s is no longer in the stack)", targetBranch, currentBranch) + if err := restoreCheckout(cfg, stateFile, s, false); err != nil { + return nil, nil, err } // Update base SHAs @@ -650,11 +246,20 @@ func rebaseRemaining(cfg *config.Config, dir string, state *StateFile, s *stack. if branch.IsMerged() { continue } - ops, err := state.Worktrees.OriginOps() + if branch.IsQueued() { + return moved, nil, fmt.Errorf("cannot rebase queued branch %s during modify", name) + } + if err := checkExpectedRef(state, name); err != nil { + return moved, nil, err + } + ops, err := branchOps(state.Worktrees, name) if err != nil { return moved, nil, err } newBase := s.ActiveBaseBranch(name) + if err := checkExpectedRef(state, newBase); err != nil { + return moved, nil, err + } oldBase := state.OriginalRefs[name] if oldBase == "" { oldBase, err = ops.MergeBase(newBase, name) @@ -672,37 +277,14 @@ func rebaseRemaining(cfg *config.Config, dir string, state *StateFile, s *stack. return moved, nil, fmt.Errorf("finding merge base for %s: %w", name, err) } if base == oldBase { + state.RemainingBranches = append([]string{}, branches[i+1:]...) continue } } - state.ConflictBranch = name - state.ConflictType = "rebase" state.RemainingBranches = append([]string{}, branches[i+1:]...) state.AffectsPRs = state.AffectsPRs || branch.PullRequest != nil - ops, err = startRefMutation(dir, state, name) - if err != nil { - return moved, nil, err - } - if err := ops.RebaseOnto(newBase, oldBase, name, git.RebaseOpts{}); err != nil { - state.Phase = PhaseConflict - if git.IsRebaseStartError(err) { - state.ConflictType = "rebase_start" - } - if saveErr := saveProgress(dir, state, s, sf); saveErr != nil { - return moved, nil, errors.Join(err, saveErr) - } - if git.IsRebaseStartError(err) { - return moved, nil, fmt.Errorf("could not start rebase of %s onto %s in %s: %w", name, newBase, state.Worktrees.Origin.Path, err) - } - files, fileErr := ops.ConflictedFiles() - if fileErr != nil { - return moved, nil, errors.Join(err, fmt.Errorf("reading conflicts in %s: %w", state.Worktrees.Origin.Path, fileErr)) - } - return moved, &modifyview.ConflictInfo{Branch: name, ConflictedFiles: files}, - fmt.Errorf("rebase conflict on %s in %s", name, state.Worktrees.Origin.Path) - } - if err := state.Worktrees.Record(name); err != nil { - return moved, nil, err + if conflict, err := rebaseInOwner(dir, state, s, sf, name, newBase, oldBase); err != nil { + return moved, conflict, err } state.ConflictBranch, state.ConflictType = "", "cascade" if err := saveProgress(dir, state, s, sf); err != nil { @@ -714,6 +296,42 @@ func rebaseRemaining(cfg *config.Config, dir string, state *StateFile, s *stack. return moved, nil, nil } +func rebaseInOwner(dir string, state *StateFile, s *stack.Stack, sf *stack.StackFile, name, newBase, oldBase string) (*modifyview.ConflictInfo, error) { + state.ConflictBranch, state.ConflictType = name, "rebase" + _, err := startRefMutation(dir, state, name) + if err != nil { + return nil, err + } + ops, err := state.Worktrees.Prepare(name) + if err != nil { + state.Phase, state.ConflictType = PhaseConflict, "rebase_start" + return nil, errors.Join(err, saveProgress(dir, state, s, sf)) + } + if err := ops.RebaseOnto(newBase, oldBase, name, git.RebaseOpts{}); err != nil { + state.Phase = PhaseConflict + if git.IsRebaseStartError(err) { + state.ConflictType = "rebase_start" + } + if saveErr := saveNativeConflict(dir, state, s, sf, ops); saveErr != nil { + return nil, errors.Join(err, saveErr) + } + if git.IsRebaseStartError(err) { + return nil, fmt.Errorf("could not start rebase of %s onto %s in %s: %w", name, newBase, state.Worktrees.Location(name).Path, err) + } + files, fileErr := ops.ConflictedFiles() + if fileErr != nil { + return nil, errors.Join(err, fmt.Errorf("reading conflicts in %s: %w", state.Worktrees.Location(name).Path, fileErr)) + } + return &modifyview.ConflictInfo{Branch: name, ConflictedFiles: files}, + fmt.Errorf("rebase conflict on %s in %s", name, state.Worktrees.Location(name).Path) + } + if err := state.Worktrees.Record(name); err != nil { + return nil, err + } + state.PendingHead = "" + return nil, nil +} + // resolveCheckoutBranch determines which branch to check out after a modify // operation completes. If the user's original branch was dropped, folded, or // renamed, this returns the most appropriate surviving branch. @@ -850,11 +468,16 @@ func ContinueApply( if state.StackBranches != nil && (state.StackName != s.Trunk.Branch || !slices.Equal(state.StackBranches, s.BranchNames())) { return fmt.Errorf("the modify catalog update did not complete or the stack changed; run `gh stack modify --abort` to recover") } + for _, branch := range []string{state.ConflictBranch, nativeBranch(state)} { + if index := s.IndexOf(branch); index >= 0 && s.Branches[index].IsSkipped() { + return fmt.Errorf("cannot continue modify on merged or queued branch %s; run `gh stack modify --abort`", branch) + } + } ctx, err := recoveryContext(gitDir, state) if err != nil { return err } - ops, err := ctx.OriginOps() + ops, err := originOps(ctx) if err != nil { return err } @@ -863,6 +486,62 @@ func ContinueApply( return err } } + normalizingFold := state.PendingAction != nil && state.PendingAction.Type == "fold_rebase" + if state.PendingAction != nil && state.PendingAction.Type != "fold_down" && !normalizingFold { + return fmt.Errorf("modify was interrupted during %s; run `gh stack modify --abort` to recover", state.PendingAction.Type) + } + conflictType := state.ConflictType + if normalizingFold { + if state.DesiredOrder == nil || state.NextAction < 0 || state.NextAction >= len(state.Execution) || + state.Execution[state.NextAction] != *state.PendingAction || + (conflictType != "rebase" && conflictType != "rebase_start") || + state.PendingAction.Branch != nativeBranch(state) { + return fmt.Errorf("fold normalization does not match the pending action; recovery state was retained") + } + } + resumeActions := state.DesiredOrder != nil && (conflictType == "cherry_pick" || conflictType == "actions" || normalizingFold) + pending := "" + path := ctx.Origin.Path + if conflictType == "cherry_pick" || conflictType == "rebase" || conflictType == "" { + ops, path, err = ConflictOps(state) + if err != nil { + return err + } + active := ops.IsRebaseInProgress() + if conflictType == "cherry_pick" { + active = ops.IsCherryPickInProgress() + if state.DesiredOrder != nil { + if state.NextAction < 0 || state.NextAction >= len(state.Execution) { + return fmt.Errorf("fold conflict has invalid action progress; recovery state was retained") + } + action := state.Execution[state.NextAction] + if action.Type != "fold_down" || action.Branch != state.FoldBranch || action.Target != state.FoldTarget { + return fmt.Errorf("fold conflict does not match the pending action; recovery state was retained") + } + } + } + if active { + pending = nativeBranch(state) + } else if state.DesiredOrder != nil { + return fmt.Errorf("the recorded native operation is no longer active in %s; use `gh stack modify --abort` to recover", path) + } + if err := checkPendingHead(state, ops); err != nil { + return err + } + if index := s.IndexOf(nativeBranch(state)); index >= 0 && !normalizingFold { + if err := checkExpectedRef(state, s.ActiveBaseBranch(nativeBranch(state))); err != nil { + return err + } + } + if conflictType == "cherry_pick" { + if err := checkExpectedRef(state, state.FoldBranch); err != nil { + return err + } + } + } + if err := preflightBranches(state, remainingTargets(state), pending); err != nil { + return err + } state.RecordStack(s) if err := SaveState(gitDir, state); err != nil { return err @@ -880,7 +559,8 @@ func ContinueApply( switch state.ConflictType { case "cherry_pick": if err := ops.CherryPickContinue(); err != nil { - return fmt.Errorf("cherry-pick continue failed in %s — resolve remaining conflicts and try again: %w", ctx.Origin.Path, err) + return errors.Join(fmt.Errorf("cherry-pick continue failed in %s — resolve remaining conflicts and try again: %w", path, err), + saveNativeConflict(gitDir, state, s, sf, ops)) } if err := ctx.Record(state.FoldTarget); err != nil { return err @@ -892,54 +572,59 @@ func ContinueApply( if foldIdx >= 0 && foldIdx < len(s.Branches) { s.Branches = append(s.Branches[:foldIdx], s.Branches[foldIdx+1:]...) } + if state.DesiredOrder != nil { + state.NextAction++ + } case "", "rebase": // Rebase conflict if ops.IsRebaseInProgress() { if err := ops.RebaseContinue(git.RebaseOpts{}); err != nil { - return fmt.Errorf("rebase continue failed in %s — resolve remaining conflicts and try again: %w", ctx.Origin.Path, err) + return errors.Join(fmt.Errorf("rebase continue failed in %s — resolve remaining conflicts and try again: %w", path, err), + saveNativeConflict(gitDir, state, s, sf, ops)) } } if err := ctx.Record(state.ConflictBranch); err != nil { return err } + if normalizingFold { + state.NextAction++ + } cfg.Successf("Rebased %s", state.ConflictBranch) case "rebase_start": - remainingBranches = append([]string{state.ConflictBranch}, remainingBranches...) - case "cascade": + if !normalizingFold { + remainingBranches = append([]string{state.ConflictBranch}, remainingBranches...) + } + case "actions", "cascade": default: return fmt.Errorf("unknown modify conflict type %q", state.ConflictType) } state.ConflictBranch, state.ConflictType = "", "cascade" + state.PendingAction, state.PendingHead = nil, "" state.RemainingBranches = remainingBranches + if resumeActions { + state.ConflictType = "actions" + } if err := saveProgress(gitDir, state, s, sf); err != nil { return err } - if _, conflict, err := rebaseRemaining(cfg, gitDir, state, s, sf, remainingBranches); err != nil { - if conflict != nil { - cfg.Warningf("Conflict rebasing %s in %s", conflict.Branch, ctx.Origin.Path) - for _, file := range conflict.ConflictedFiles { - cfg.Printf(" %s", file) - } - cfg.Printf("") - cfg.Printf("Resolve the conflicts in %s, stage with `%s`, then run `%s`", - ctx.Origin.Path, - cfg.ColorCyan("git add "), - cfg.ColorCyan("gh stack modify --continue")) - cfg.Printf("Or restore the stack with `%s`", - cfg.ColorCyan("gh stack modify --abort")) + if resumeActions { + result := &modifyview.ApplyResult{Success: true} + if conflict, err := runActions(cfg, gitDir, state, s, sf, result); err != nil { + printContinueConflict(cfg, state, conflict) + return err } + remainingBranches = state.RemainingBranches + } + if _, conflict, err := rebaseRemaining(cfg, gitDir, state, s, sf, remainingBranches); err != nil { + printContinueConflict(cfg, state, conflict) return err } - // All rebases done — check out the best branch - if state.OriginalBranch != "" { - targetBranch := resolveCheckoutBranch(state.OriginalBranch, state.Plan, state.Snapshot, s) - if err := ctx.RestoreOrigin(targetBranch); err != nil { - return err - } - if targetBranch != state.OriginalBranch { - cfg.Printf("Switched to %s (original branch %s is no longer in the stack)", targetBranch, state.OriginalBranch) - } + if err := checkResultRefs(state, s); err != nil { + return err + } + if err := restoreCheckout(cfg, state, s, false); err != nil { + return err } // Update base SHAs @@ -960,6 +645,21 @@ func ContinueApply( return nil } +func printContinueConflict(cfg *config.Config, state *StateFile, conflict *modifyview.ConflictInfo) { + if conflict == nil { + return + } + path := state.Worktrees.Location(nativeBranch(state)).Path + cfg.Warningf("Conflict applying %s in %s", conflict.Branch, path) + for _, file := range conflict.ConflictedFiles { + cfg.Printf(" %s", file) + } + cfg.Printf("") + cfg.Printf("Resolve the conflicts in %s, stage with `%s`, then run `%s`", + path, cfg.ColorCyan("git add "), cfg.ColorCyan("gh stack modify --continue")) + cfg.Printf("Or restore the stack with `%s`", cfg.ColorCyan("gh stack modify --abort")) +} + // Unwind restores the stack to its pre-modify state using the snapshot. // stackIndex is retained for legacy callers, but is never used as an identity. func Unwind(cfg *config.Config, gitDir string, snapshot Snapshot, stackIndex int, sf *stack.StackFile, plan []Action) error { diff --git a/internal/modify/apply_test.go b/internal/modify/apply_test.go index b4f9474..12e9e8c 100644 --- a/internal/modify/apply_test.go +++ b/internal/modify/apply_test.go @@ -3,6 +3,8 @@ package modify import ( "encoding/json" "errors" + "fmt" + "io" "os" "os/exec" "path/filepath" @@ -320,7 +322,7 @@ func TestApplyPlan_FoldDown(t *testing.T) { return nil } mock.LogRangeFn = func(base, head string) ([]git.CommitInfo, error) { - if base == "A" && head == "B" { + if base == "sha-A" && head == "B" { return []git.CommitInfo{ {SHA: "commit-b2"}, {SHA: "commit-b1"}, @@ -2657,6 +2659,39 @@ func TestApplyPlan_SeparateGitDirOrigin(t *testing.T) { } } +func TestModifyRecovery_MissingSeparateGitDirOriginRetainsOriginalLocation(t *testing.T) { + root, linked, caller, common, sf := setupModifyWorktrees(t, false, "--separate-git-dir", t.TempDir()) + runModifyGit(t, linked, "checkout", "-q", "--detach") + runModifyGit(t, root, "checkout", "-q", "B") + originOps := git.ForWorktree(root) + callerOps := originOps.ForWorktree(caller) + restore := git.SetOps(originOps) + defer restore() + ctx, err := CheckWorktrees(&sf.Stacks[0]) + require.NoError(t, err) + snapshot, err := BuildSnapshot(&sf.Stacks[0]) + require.NoError(t, err) + state := &StateFile{ + SchemaVersion: 1, Phase: PhaseApplying, OriginalBranch: "B", + Worktrees: ctx, Snapshot: snapshot, + } + state.RecordStack(&sf.Stacks[0]) + require.NoError(t, SaveState(common, state)) + before, err := os.ReadFile(StatePath(common)) + require.NoError(t, err) + require.NoError(t, os.Rename(root, root+"-moved")) + restoreCaller := git.SetOps(callerOps) + defer restoreCaller() + cfg, _, _ := config.NewTestConfig() + defer cfg.Out.Close() + defer cfg.Err.Close() + require.Error(t, UnwindFromStateFile(cfg, common)) + after, err := os.ReadFile(StatePath(common)) + require.NoError(t, err) + assert.Equal(t, before, after, "failed origin discovery must not replace the saved location with the administration directory") + assert.Equal(t, "observer", runModifyGit(t, caller, "branch", "--show-current")) +} + func TestApplyPlan_LinkedWorktreeWithForeignTrunk(t *testing.T) { root, origin, caller, common, sf := setupModifyWorktrees(t, false) require.NoError(t, os.WriteFile(filepath.Join(caller, "note.txt"), []byte("keep"), 0644)) @@ -2680,10 +2715,11 @@ func TestApplyPlan_LinkedWorktreeWithForeignTrunk(t *testing.T) { assert.False(t, StateExists(common)) } -func TestApplyPlan_DistributedStackRejectedBeforeMutation(t *testing.T) { +func TestApplyPlan_DirtyDistributedOwnerRejectedBeforeMutation(t *testing.T) { root, origin, _, common, sf := setupModifyWorktrees(t, false) owner := filepath.Join(filepath.Dir(origin), "A owner") runModifyGit(t, root, "worktree", "add", "-q", owner, "A") + require.NoError(t, os.WriteFile(filepath.Join(owner, "uncommitted.txt"), []byte("keep"), 0644)) before, err := os.ReadFile(filepath.Join(common, "gh-stack")) require.NoError(t, err) original := runModifyGit(t, root, "rev-parse", "B") @@ -2695,7 +2731,7 @@ func TestApplyPlan_DistributedStackRejectedBeforeMutation(t *testing.T) { nodes := makeNodes(&sf.Stacks[0]) nodes[1].PendingAction = &modifyview.PendingAction{Type: modifyview.ActionRename, NewName: "new-B"} _, _, err = ApplyPlan(cfg, common, &sf.Stacks[0], sf, nodes, "B", noopUpdateBaseSHAs) - require.ErrorContains(t, err, "distributed modify is not supported yet") + require.ErrorContains(t, err, "uncommitted changes") assert.Contains(t, err.Error(), "A owner") after, err := os.ReadFile(filepath.Join(common, "gh-stack")) require.NoError(t, err) @@ -2914,6 +2950,7 @@ func TestStartRefMutation_UsesRecordedExpectedTip(t *testing.T) { state.Worktrees.Touched = map[string]string{"A": expected} } mock := newApplyMock(dir, map[string]string{"A": expected}) + mock.RootDirFn = func() (string, error) { return origin, nil } restore := git.SetOps(mock) defer restore() _, err := startRefMutation(dir, state, "A") @@ -2923,3 +2960,932 @@ func TestStartRefMutation_UsesRecordedExpectedTip(t *testing.T) { }) } } + +type distributedModifyRepo struct { + root, origin, caller, common string + owners map[string]string + refs map[string]string + sf *stack.StackFile +} + +func commitModifyFile(t *testing.T, dir, file, content, message string) { + t.Helper() + require.NoError(t, os.WriteFile(filepath.Join(dir, file), []byte(content), 0644)) + runModifyGit(t, dir, "add", file) + runModifyGit(t, dir, "commit", "-qm", message) +} + +func setupDistributedModify(t *testing.T, conflicting bool) distributedModifyRepo { + t.Helper() + root, origin, caller, common, sf := setupModifyWorktrees(t, conflicting) + runModifyGit(t, origin, "checkout", "-qb", "C") + if conflicting { + commitModifyFile(t, origin, "base.txt", "C\n", "C") + } else { + commitModifyFile(t, origin, "c.txt", "C\n", "C") + } + sf.Stacks[0].Branches = append(sf.Stacks[0].Branches, stack.BranchRef{Branch: "C"}) + require.NoError(t, stack.Save(common, sf)) + repo := distributedModifyRepo{ + root: root, origin: origin, caller: caller, common: common, sf: sf, + owners: map[string]string{"C": origin}, refs: make(map[string]string), + } + for _, branch := range []string{"A", "B"} { + path := filepath.Join(filepath.Dir(origin), "owner "+branch) + runModifyGit(t, root, "worktree", "add", "-q", path, branch) + repo.owners[branch] = path + } + for _, branch := range []string{"A", "B", "C"} { + repo.refs[branch] = runModifyGit(t, root, "rev-parse", branch) + } + return repo +} + +func insertedModifyNode(name string) modifyview.ModifyBranchNode { + return modifyview.ModifyBranchNode{ + BranchNode: stackview.BranchNode{Ref: stack.BranchRef{Branch: name}}, + OriginalPosition: -1, + IsInserted: true, + PendingAction: &modifyview.PendingAction{Type: modifyview.ActionInsertBelow, NewName: name}, + } +} + +func TestDistributedModify_AllActions(t *testing.T) { + tests := []struct { + name string + nodes func([]modifyview.ModifyBranchNode) []modifyview.ModifyBranchNode + order []string + files []string + renamedA bool + renamedB bool + }{ + { + name: "rename", + nodes: func(nodes []modifyview.ModifyBranchNode) []modifyview.ModifyBranchNode { + nodes[0].PendingAction = &modifyview.PendingAction{Type: modifyview.ActionRename, NewName: "new-A"} + return nodes + }, + order: []string{"new-A", "B", "C"}, files: []string{"a.txt", "b.txt", "base.txt", "c.txt"}, renamedA: true, + }, + { + name: "insert", + nodes: func(nodes []modifyview.ModifyBranchNode) []modifyview.ModifyBranchNode { + return []modifyview.ModifyBranchNode{nodes[0], insertedModifyNode("inserted"), nodes[1], nodes[2]} + }, + order: []string{"A", "inserted", "B", "C"}, files: []string{"a.txt", "b.txt", "base.txt", "c.txt"}, + }, + { + name: "drop", + nodes: func(nodes []modifyview.ModifyBranchNode) []modifyview.ModifyBranchNode { + nodes[0].PendingAction = &modifyview.PendingAction{Type: modifyview.ActionDrop} + nodes[0].Removed = true + return nodes + }, + order: []string{"B", "C"}, files: []string{"b.txt", "base.txt", "c.txt"}, + }, + { + name: "fold down", + nodes: func(nodes []modifyview.ModifyBranchNode) []modifyview.ModifyBranchNode { + nodes[1].PendingAction = &modifyview.PendingAction{Type: modifyview.ActionFoldDown} + nodes[1].Removed = true + return nodes + }, + order: []string{"A", "C"}, files: []string{"a.txt", "b.txt", "base.txt", "c.txt"}, + }, + { + name: "fold up", + nodes: func(nodes []modifyview.ModifyBranchNode) []modifyview.ModifyBranchNode { + nodes[1].PendingAction = &modifyview.PendingAction{Type: modifyview.ActionFoldUp} + nodes[1].Removed = true + return nodes + }, + order: []string{"A", "C"}, files: []string{"a.txt", "b.txt", "base.txt", "c.txt"}, + }, + { + name: "reorder", + nodes: func(nodes []modifyview.ModifyBranchNode) []modifyview.ModifyBranchNode { + return []modifyview.ModifyBranchNode{nodes[2], nodes[1], nodes[0]} + }, + order: []string{"C", "B", "A"}, files: []string{"a.txt", "b.txt", "base.txt", "c.txt"}, + }, + { + name: "rename insert and drop", + nodes: func(nodes []modifyview.ModifyBranchNode) []modifyview.ModifyBranchNode { + nodes[0].PendingAction = &modifyview.PendingAction{Type: modifyview.ActionDrop} + nodes[0].Removed = true + nodes[1].PendingAction = &modifyview.PendingAction{Type: modifyview.ActionRename, NewName: "new-B"} + return []modifyview.ModifyBranchNode{nodes[0], nodes[1], insertedModifyNode("inserted"), nodes[2]} + }, + order: []string{"new-B", "inserted", "C"}, files: []string{"b.txt", "base.txt", "c.txt"}, renamedB: true, + }, + } + for _, tt := range tests { + for _, unoccupied := range []bool{false, true} { + layout := "occupied" + if unoccupied { + layout = "mixed" + } + t.Run(tt.name+"/"+layout, func(t *testing.T) { + repo := setupDistributedModify(t, false) + if unoccupied { + runModifyGit(t, repo.owners["B"], "checkout", "-qb", "b-observer", "main") + require.NoError(t, os.WriteFile(filepath.Join(repo.owners["B"], "unrelated.txt"), []byte("keep"), 0644)) + } + require.NoError(t, os.WriteFile(filepath.Join(repo.caller, "unrelated.txt"), []byte("keep"), 0644)) + beforeTrees := runModifyGit(t, repo.root, "worktree", "list", "--porcelain") + restore := git.SetOps(git.ForWorktree(repo.origin)) + defer restore() + cfg, _, _ := config.NewTestConfig() + defer cfg.Out.Close() + defer cfg.Err.Close() + result, conflict, err := ApplyPlan(cfg, repo.common, &repo.sf.Stacks[0], repo.sf, + tt.nodes(makeNodes(&repo.sf.Stacks[0])), "C", noopUpdateBaseSHAs) + require.NoError(t, err) + assert.Nil(t, conflict) + require.NotNil(t, result) + saved, err := stack.Load(repo.common) + require.NoError(t, err) + assert.Equal(t, tt.order, saved.Stacks[0].BranchNames()) + top := tt.order[len(tt.order)-1] + assert.Equal(t, tt.files, strings.Fields(runModifyGit(t, repo.root, "ls-tree", "--name-only", top))) + for i, branch := range tt.order { + parent := "main" + if i > 0 { + parent = tt.order[i-1] + } + runModifyGit(t, repo.root, "merge-base", "--is-ancestor", parent, branch) + } + aName, bName := "A", "B" + if tt.renamedA { + aName = "new-A" + } + if tt.renamedB { + bName = "new-B" + } + if unoccupied { + bName = "b-observer" + assert.Equal(t, "?? unrelated.txt", runModifyGit(t, repo.owners["B"], "status", "--porcelain")) + } + assert.Equal(t, aName, runModifyGit(t, repo.owners["A"], "branch", "--show-current")) + assert.Equal(t, bName, runModifyGit(t, repo.owners["B"], "branch", "--show-current")) + assert.Equal(t, "C", runModifyGit(t, repo.origin, "branch", "--show-current")) + assert.Equal(t, "observer", runModifyGit(t, repo.caller, "branch", "--show-current")) + afterTrees := runModifyGit(t, repo.root, "worktree", "list", "--porcelain") + assert.Equal(t, strings.Count(beforeTrees, "worktree "), strings.Count(afterTrees, "worktree ")) + if tt.name == "drop" { + assert.Equal(t, repo.refs["A"], runModifyGit(t, repo.root, "rev-parse", "A")) + } + if tt.name == "fold down" || tt.name == "fold up" { + assert.Equal(t, repo.refs["B"], runModifyGit(t, repo.root, "rev-parse", "B")) + } + if tt.name == "rename insert and drop" { + assert.Equal(t, runModifyGit(t, repo.root, "rev-parse", "new-B"), runModifyGit(t, repo.root, "rev-parse", "inserted"), + "an inserted empty layer must not resurrect the dropped parent's commits") + } + assert.False(t, StateExists(repo.common)) + }) + } + } +} + +func TestDistributedModify_RetainsOriginForForeignSurvivor(t *testing.T) { + for _, action := range []modifyview.ActionType{modifyview.ActionDrop, modifyview.ActionFoldDown} { + t.Run(string(action), func(t *testing.T) { + repo := setupDistributedModify(t, false) + restore := git.SetOps(git.ForWorktree(repo.origin)) + defer restore() + cfg, _, errR := config.NewTestConfig() + nodes := makeNodes(&repo.sf.Stacks[0]) + nodes[2].PendingAction = &modifyview.PendingAction{Type: action} + nodes[2].Removed = true + _, _, err := ApplyPlan(cfg, repo.common, &repo.sf.Stacks[0], repo.sf, nodes, "C", noopUpdateBaseSHAs) + cfg.Out.Close() + cfg.Err.Close() + require.NoError(t, err) + output, err := io.ReadAll(errR) + require.NoError(t, err) + assert.Contains(t, string(output), repo.owners["B"]) + assert.Contains(t, string(output), "Kept C") + assert.Equal(t, "C", runModifyGit(t, repo.origin, "branch", "--show-current")) + assert.Equal(t, repo.refs["C"], runModifyGit(t, repo.root, "rev-parse", "C")) + assert.Equal(t, "B", runModifyGit(t, repo.owners["B"], "branch", "--show-current")) + assert.Equal(t, []string{"A", "B"}, repo.sf.Stacks[0].BranchNames()) + }) + } +} + +func TestDistributedModify_PreflightsAllTargets(t *testing.T) { + for _, condition := range []string{"dirty", "busy"} { + t.Run(condition, func(t *testing.T) { + repo := setupDistributedModify(t, false) + path := filepath.Join(repo.owners["B"], "uncommitted.txt") + message := "uncommitted changes" + if condition == "busy" { + dir := runModifyGit(t, repo.owners["B"], "rev-parse", "--absolute-git-dir") + path, message = filepath.Join(dir, "MERGE_HEAD"), "Git operation" + } + require.NoError(t, os.WriteFile(path, []byte("keep\n"), 0644)) + before, err := os.ReadFile(filepath.Join(repo.common, "gh-stack")) + require.NoError(t, err) + restore := git.SetOps(git.ForWorktree(repo.origin)) + defer restore() + cfg, _, _ := config.NewTestConfig() + defer cfg.Out.Close() + defer cfg.Err.Close() + nodes := makeNodes(&repo.sf.Stacks[0]) + nodes[0].PendingAction = &modifyview.PendingAction{Type: modifyview.ActionRename, NewName: "new-A"} + _, _, err = ApplyPlan(cfg, repo.common, &repo.sf.Stacks[0], repo.sf, nodes, "C", noopUpdateBaseSHAs) + require.ErrorContains(t, err, message) + assert.Contains(t, err.Error(), repo.owners["B"]) + for branch, sha := range repo.refs { + assert.Equal(t, sha, runModifyGit(t, repo.root, "rev-parse", branch)) + } + after, err := os.ReadFile(filepath.Join(repo.common, "gh-stack")) + require.NoError(t, err) + assert.Equal(t, before, after) + assert.False(t, StateExists(repo.common)) + }) + } +} + +func TestDistributedModify_DropLeavesDirtySourceUntouched(t *testing.T) { + repo := setupDistributedModify(t, false) + require.NoError(t, os.WriteFile(filepath.Join(repo.owners["B"], "uncommitted.txt"), []byte("keep"), 0644)) + restore := git.SetOps(git.ForWorktree(repo.origin)) + defer restore() + cfg, _, _ := config.NewTestConfig() + defer cfg.Out.Close() + defer cfg.Err.Close() + nodes := makeNodes(&repo.sf.Stacks[0]) + nodes[1].PendingAction = &modifyview.PendingAction{Type: modifyview.ActionDrop} + nodes[1].Removed = true + _, _, err := ApplyPlan(cfg, repo.common, &repo.sf.Stacks[0], repo.sf, nodes, "C", noopUpdateBaseSHAs) + require.NoError(t, err) + assert.Equal(t, repo.refs["B"], runModifyGit(t, repo.root, "rev-parse", "B")) + assert.Equal(t, "?? uncommitted.txt", runModifyGit(t, repo.owners["B"], "status", "--porcelain")) + assert.Equal(t, "B", runModifyGit(t, repo.owners["B"], "branch", "--show-current")) +} + +func TestDistributedModify_RebaseRecovery(t *testing.T) { + for _, scenario := range []string{"foreign conflicts", "same-tree remaining branch", "moved owner", "abort renamed owner"} { + t.Run(scenario, func(t *testing.T) { + repo := setupDistributedModify(t, true) + if scenario == "same-tree remaining branch" { + runModifyGit(t, repo.owners["B"], "checkout", "-qb", "b-observer", "main") + } + originOps := git.ForWorktree(repo.origin) + restore := git.SetOps(originOps) + defer restore() + cfg, _, _ := config.NewTestConfig() + defer cfg.Out.Close() + defer cfg.Err.Close() + nodes := makeNodes(&repo.sf.Stacks[0]) + nodes[0].PendingAction = &modifyview.PendingAction{Type: modifyview.ActionDrop} + nodes[0].Removed = true + bName := "B" + if scenario == "abort renamed owner" { + bName = "new-B" + nodes[1].PendingAction = &modifyview.PendingAction{Type: modifyview.ActionRename, NewName: bName} + } + _, conflict, err := ApplyPlan(cfg, repo.common, &repo.sf.Stacks[0], repo.sf, nodes, "C", noopUpdateBaseSHAs) + require.Error(t, err) + require.NotNil(t, conflict) + assert.Equal(t, bName, conflict.Branch) + state, err := LoadState(repo.common) + require.NoError(t, err) + require.NotNil(t, state) + bPath := repo.owners["B"] + if scenario == "same-tree remaining branch" { + bPath = repo.origin + } + assert.True(t, worktree.SamePath(state.Worktrees.Location(bName).Path, bPath)) + assert.True(t, originOps.ForWorktree(bPath).IsRebaseInProgress()) + if scenario == "moved owner" { + moved := bPath + "-moved" + runModifyGit(t, repo.root, "worktree", "move", bPath, moved) + bPath, repo.owners["B"] = moved, moved + } + restoreCaller := git.SetOps(originOps.ForWorktree(repo.caller)) + defer restoreCaller() + if scenario == "abort renamed owner" { + require.NoError(t, UnwindFromStateFile(cfg, repo.common)) + assert.Equal(t, "B", runModifyGit(t, repo.owners["B"], "branch", "--show-current")) + for branch, sha := range repo.refs { + assert.Equal(t, sha, runModifyGit(t, repo.root, "rev-parse", branch)) + } + } else { + require.NoError(t, os.WriteFile(filepath.Join(bPath, "base.txt"), []byte("resolved B\n"), 0644)) + runModifyGit(t, bPath, "add", "base.txt") + require.ErrorContains(t, ContinueApply(cfg, repo.common, noopUpdateBaseSHAs), "rebase conflict on C") + next, err := LoadState(repo.common) + require.NoError(t, err) + assert.Equal(t, "C", next.Worktrees.Pending) + assert.True(t, worktree.SamePath(next.Worktrees.Location("C").Path, repo.origin)) + require.NoError(t, os.WriteFile(filepath.Join(repo.origin, "base.txt"), []byte("resolved C\n"), 0644)) + runModifyGit(t, repo.origin, "add", "base.txt") + restoreAnother := git.SetOps(originOps.ForWorktree(repo.owners["A"])) + defer restoreAnother() + require.NoError(t, ContinueApply(cfg, repo.common, noopUpdateBaseSHAs)) + saved, err := stack.Load(repo.common) + require.NoError(t, err) + assert.Equal(t, []string{"B", "C"}, saved.Stacks[0].BranchNames()) + runModifyGit(t, repo.root, "merge-base", "--is-ancestor", "B", "C") + } + assert.Equal(t, "A", runModifyGit(t, repo.owners["A"], "branch", "--show-current")) + assert.Equal(t, "C", runModifyGit(t, repo.origin, "branch", "--show-current")) + assert.Equal(t, "observer", runModifyGit(t, repo.caller, "branch", "--show-current")) + assert.False(t, StateExists(repo.common)) + }) + } +} + +func setupDistributedFoldConflict(t *testing.T) distributedModifyRepo { + t.Helper() + repo := setupDistributedModify(t, true) + runModifyGit(t, repo.origin, "checkout", "-qb", "D") + commitModifyFile(t, repo.origin, "base.txt", "D\n", "D") + path := filepath.Join(filepath.Dir(repo.origin), "owner C") + runModifyGit(t, repo.root, "worktree", "add", "-q", path, "C") + repo.owners["C"], repo.owners["D"] = path, repo.origin + repo.refs["D"] = runModifyGit(t, repo.root, "rev-parse", "D") + repo.sf.Stacks[0].Branches = append(repo.sf.Stacks[0].Branches, stack.BranchRef{Branch: "D"}) + require.NoError(t, stack.Save(repo.common, repo.sf)) + return repo +} + +func distributedFoldConflictNodes(s *stack.Stack) []modifyview.ModifyBranchNode { + nodes := makeNodes(s) + nodes[1].PendingAction = &modifyview.PendingAction{Type: modifyview.ActionDrop} + nodes[1].Removed = true + nodes[2].PendingAction = &modifyview.PendingAction{Type: modifyview.ActionFoldDown} + nodes[2].Removed = true + nodes[3].PendingAction = &modifyview.PendingAction{Type: modifyview.ActionRename, NewName: "new-D"} + return append(nodes, insertedModifyNode("inserted")) +} + +func TestDistributedModify_FoldThenRebaseRecovery(t *testing.T) { + for _, outcome := range []string{"continue", "abort"} { + t.Run(outcome, func(t *testing.T) { + repo := setupDistributedFoldConflict(t) + originOps := git.ForWorktree(repo.origin) + restore := git.SetOps(originOps) + defer restore() + cfg, _, _ := config.NewTestConfig() + defer cfg.Out.Close() + defer cfg.Err.Close() + _, conflict, err := ApplyPlan(cfg, repo.common, &repo.sf.Stacks[0], repo.sf, + distributedFoldConflictNodes(&repo.sf.Stacks[0]), "D", noopUpdateBaseSHAs) + require.Error(t, err) + require.NotNil(t, conflict) + assert.Equal(t, "C", conflict.Branch) + state, err := LoadState(repo.common) + require.NoError(t, err) + assert.Equal(t, "cherry_pick", state.ConflictType) + assert.Equal(t, "A", state.Worktrees.Pending) + assert.Equal(t, 2, state.NextAction, "rename and insertion must already be checkpointed") + assert.True(t, worktree.SamePath(state.Worktrees.Location("A").Path, repo.owners["A"])) + restoreCaller := git.SetOps(originOps.ForWorktree(repo.caller)) + defer restoreCaller() + require.NoError(t, os.WriteFile(filepath.Join(repo.owners["A"], "base.txt"), []byte("resolved C\n"), 0644)) + runModifyGit(t, repo.owners["A"], "add", "base.txt") + dirtyPath := filepath.Join(repo.origin, "unrelated.txt") + require.NoError(t, os.WriteFile(dirtyPath, []byte("keep"), 0644)) + require.ErrorContains(t, ContinueApply(cfg, repo.common, noopUpdateBaseSHAs), "uncommitted changes") + assert.True(t, originOps.ForWorktree(repo.owners["A"]).IsCherryPickInProgress(), + "other target worktrees must be preflighted before finishing the pending native operation") + require.NoError(t, os.Remove(dirtyPath)) + require.ErrorContains(t, ContinueApply(cfg, repo.common, noopUpdateBaseSHAs), "rebase conflict on new-D") + state, err = LoadState(repo.common) + require.NoError(t, err) + assert.Equal(t, "rebase", state.ConflictType) + assert.Equal(t, "new-D", state.Worktrees.Pending) + assert.Equal(t, len(state.Execution), state.NextAction) + saved, err := stack.Load(repo.common) + require.NoError(t, err) + assert.Equal(t, []string{"A", "new-D", "inserted"}, saved.Stacks[0].BranchNames(), + "continuation must run the drop after the fold, not resurrect its source branches") + if outcome == "abort" { + require.NoError(t, UnwindFromStateFile(cfg, repo.common)) + for branch, sha := range repo.refs { + assert.Equal(t, sha, runModifyGit(t, repo.root, "rev-parse", branch)) + } + assert.False(t, originOps.BranchExists("new-D")) + assert.False(t, originOps.BranchExists("inserted")) + assert.Equal(t, "D", runModifyGit(t, repo.origin, "branch", "--show-current")) + } else { + require.NoError(t, os.WriteFile(filepath.Join(repo.origin, "base.txt"), []byte("resolved D\n"), 0644)) + runModifyGit(t, repo.origin, "add", "base.txt") + restoreSource := git.SetOps(originOps.ForWorktree(repo.owners["C"])) + defer restoreSource() + require.NoError(t, ContinueApply(cfg, repo.common, noopUpdateBaseSHAs)) + assert.Equal(t, "new-D", runModifyGit(t, repo.origin, "branch", "--show-current")) + assert.Equal(t, runModifyGit(t, repo.root, "rev-parse", "new-D"), runModifyGit(t, repo.root, "rev-parse", "inserted")) + } + for _, branch := range []string{"A", "B", "C"} { + assert.Equal(t, branch, runModifyGit(t, repo.owners[branch], "branch", "--show-current")) + } + for _, branch := range []string{"B", "C"} { + assert.Equal(t, repo.refs[branch], runModifyGit(t, repo.root, "rev-parse", branch), + "dropped/folded source refs must remain intact") + } + assert.Equal(t, "observer", runModifyGit(t, repo.caller, "branch", "--show-current")) + assert.False(t, StateExists(repo.common)) + }) + } +} + +func TestDistributedModify_PreservesNewCommitOnRemainingOwner(t *testing.T) { + repo := setupDistributedModify(t, true) + originOps := git.ForWorktree(repo.origin) + restore := git.SetOps(originOps) + defer restore() + cfg, _, _ := config.NewTestConfig() + defer cfg.Out.Close() + defer cfg.Err.Close() + nodes := makeNodes(&repo.sf.Stacks[0]) + nodes[0].PendingAction = &modifyview.PendingAction{Type: modifyview.ActionDrop} + nodes[0].Removed = true + _, conflict, err := ApplyPlan(cfg, repo.common, &repo.sf.Stacks[0], repo.sf, nodes, "C", noopUpdateBaseSHAs) + require.Error(t, err) + require.NotNil(t, conflict) + commitModifyFile(t, repo.origin, "external.txt", "keep this\n", "external commit while modify paused") + external := runModifyGit(t, repo.root, "rev-parse", "C") + restoreCaller := git.SetOps(originOps.ForWorktree(repo.caller)) + defer restoreCaller() + require.ErrorContains(t, ContinueApply(cfg, repo.common, noopUpdateBaseSHAs), "C changed since") + assert.True(t, originOps.ForWorktree(repo.owners["B"]).IsRebaseInProgress()) + saved, err := LoadState(repo.common) + require.NoError(t, err) + assert.NotContains(t, saved.Worktrees.Touched, "C") + require.NoError(t, UnwindFromStateFile(cfg, repo.common)) + assert.Equal(t, external, runModifyGit(t, repo.root, "rev-parse", "C")) + assert.Equal(t, repo.refs["B"], runModifyGit(t, repo.root, "rev-parse", "B")) + assert.False(t, StateExists(repo.common)) +} + +func TestDistributedModify_MetadataSaveFailureRetainsRecovery(t *testing.T) { + repo := setupDistributedModify(t, false) + originOps := git.ForWorktree(repo.origin) + restore := git.SetOps(originOps) + defer restore() + cfg, _, _ := config.NewTestConfig() + defer cfg.Out.Close() + defer cfg.Err.Close() + nodes := makeNodes(&repo.sf.Stacks[0]) + nodes[0].PendingAction = &modifyview.PendingAction{Type: modifyview.ActionRename, NewName: "new-A"} + _, _, err := ApplyPlan(cfg, repo.common, &repo.sf.Stacks[0], repo.sf, nodes, "C", func(*stack.Stack) { + external, err := stack.Load(repo.common) + require.NoError(t, err) + external.Stacks = append(external.Stacks, stack.Stack{ + Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "observer"}}, + }) + require.NoError(t, stack.Save(repo.common, external)) + }) + var stale *stack.StaleError + require.ErrorAs(t, err, &stale) + saved, err := LoadState(repo.common) + require.NoError(t, err) + require.NotNil(t, saved) + assert.Equal(t, PhaseApplying, saved.Phase) + assert.Equal(t, "new-A", runModifyGit(t, repo.owners["A"], "branch", "--show-current")) + restoreCaller := git.SetOps(originOps.ForWorktree(repo.caller)) + defer restoreCaller() + require.NoError(t, UnwindFromStateFile(cfg, repo.common)) + assert.Equal(t, "A", runModifyGit(t, repo.owners["A"], "branch", "--show-current")) + assert.Equal(t, "C", runModifyGit(t, repo.origin, "branch", "--show-current")) + final, err := stack.Load(repo.common) + require.NoError(t, err) + require.Len(t, final.Stacks, 2) + assert.Equal(t, []string{"A", "B", "C"}, final.Stacks[0].BranchNames()) + assert.Equal(t, []string{"observer"}, final.Stacks[1].BranchNames()) + assert.False(t, StateExists(repo.common)) +} + +func TestDistributedModify_AbortPreservesChangedPendingRef(t *testing.T) { + repo := setupDistributedModify(t, true) + originOps := git.ForWorktree(repo.origin) + restore := git.SetOps(originOps) + defer restore() + cfg, _, _ := config.NewTestConfig() + defer cfg.Out.Close() + defer cfg.Err.Close() + nodes := makeNodes(&repo.sf.Stacks[0]) + nodes[0].PendingAction = &modifyview.PendingAction{Type: modifyview.ActionDrop} + nodes[0].Removed = true + _, conflict, err := ApplyPlan(cfg, repo.common, &repo.sf.Stacks[0], repo.sf, nodes, "C", noopUpdateBaseSHAs) + require.Error(t, err) + require.NotNil(t, conflict) + tree := runModifyGit(t, repo.root, "rev-parse", "B^{tree}") + external := runModifyGit(t, repo.root, "commit-tree", tree, "-p", "B", "-m", "external commit") + runModifyGit(t, repo.root, "update-ref", "refs/heads/B", external) + restoreCaller := git.SetOps(originOps.ForWorktree(repo.caller)) + defer restoreCaller() + require.ErrorContains(t, ContinueApply(cfg, repo.common, noopUpdateBaseSHAs), "changed after modify paused") + require.ErrorContains(t, UnwindFromStateFile(cfg, repo.common), "changed after modify paused") + assert.Equal(t, external, runModifyGit(t, repo.root, "rev-parse", "B")) + assert.True(t, originOps.ForWorktree(repo.owners["B"]).IsRebaseInProgress(), + "abort must reject the changed branch ref before native Git can reset it") + assert.True(t, StateExists(repo.common)) +} + +func TestDistributedModify_MultipleFoldsKeepOriginalCutoffs(t *testing.T) { + for _, direction := range []modifyview.ActionType{modifyview.ActionFoldDown, modifyview.ActionFoldUp} { + t.Run(string(direction), func(t *testing.T) { + repo := setupDistributedModify(t, false) + original := "C" + nodes := makeNodes(&repo.sf.Stacks[0]) + if direction == modifyview.ActionFoldDown { + nodes[1].PendingAction = &modifyview.PendingAction{Type: direction} + nodes[1].Removed = true + nodes[2].PendingAction = &modifyview.PendingAction{Type: direction} + nodes[2].Removed = true + } else { + nodes[0].PendingAction = &modifyview.PendingAction{Type: direction} + nodes[0].Removed = true + nodes[1].PendingAction = &modifyview.PendingAction{Type: direction} + nodes[1].Removed = true + } + restore := git.SetOps(git.ForWorktree(repo.origin)) + defer restore() + cfg, _, _ := config.NewTestConfig() + defer cfg.Out.Close() + defer cfg.Err.Close() + _, _, err := ApplyPlan(cfg, repo.common, &repo.sf.Stacks[0], repo.sf, nodes, original, noopUpdateBaseSHAs) + require.NoError(t, err) + receiver := "C" + if direction == modifyview.ActionFoldDown { + receiver = "A" + } + assert.Equal(t, []string{receiver}, repo.sf.Stacks[0].BranchNames()) + assert.Equal(t, []string{"a.txt", "b.txt", "base.txt", "c.txt"}, + strings.Fields(runModifyGit(t, repo.root, "ls-tree", "--name-only", receiver))) + assert.Equal(t, "C", runModifyGit(t, repo.origin, "branch", "--show-current")) + assert.Equal(t, "A", runModifyGit(t, repo.owners["A"], "branch", "--show-current")) + assert.Equal(t, "B", runModifyGit(t, repo.owners["B"], "branch", "--show-current")) + }) + } +} + +func TestDistributedModify_MergedOwnerIsNotTouched(t *testing.T) { + repo := setupDistributedModify(t, false) + repo.sf.Stacks[0].Branches[0].PullRequest = &stack.PullRequestRef{Number: 1, Merged: true} + require.NoError(t, stack.Save(repo.common, repo.sf)) + require.NoError(t, os.WriteFile(filepath.Join(repo.owners["A"], "uncommitted.txt"), []byte("keep"), 0644)) + restore := git.SetOps(git.ForWorktree(repo.origin)) + defer restore() + cfg, _, _ := config.NewTestConfig() + defer cfg.Out.Close() + defer cfg.Err.Close() + nodes := makeNodes(&repo.sf.Stacks[0]) + nodes[2].PendingAction = &modifyview.PendingAction{Type: modifyview.ActionRename, NewName: "new-C"} + _, _, err := ApplyPlan(cfg, repo.common, &repo.sf.Stacks[0], repo.sf, nodes, "C", noopUpdateBaseSHAs) + require.NoError(t, err) + assert.Equal(t, repo.refs["A"], runModifyGit(t, repo.root, "rev-parse", "A")) + assert.Equal(t, "?? uncommitted.txt", runModifyGit(t, repo.owners["A"], "status", "--porcelain")) + assert.Equal(t, "A", runModifyGit(t, repo.owners["A"], "branch", "--show-current")) +} + +func TestUnwind_RetryAfterCreatedRefCleanupSaveFailure(t *testing.T) { + dir, origin := t.TempDir(), t.TempDir() + original := stack.Stack{Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "A"}}} + modified := original + modified.Branches = append([]stack.BranchRef{}, original.Branches...) + modified.Branches = append(modified.Branches, stack.BranchRef{Branch: "inserted"}) + writeTestStackFile(t, dir, modified) + metadata, err := json.Marshal(original) + require.NoError(t, err) + state := &StateFile{ + SchemaVersion: 1, Phase: PhaseApplying, OriginalBranch: "A", + Snapshot: Snapshot{StackMetadata: metadata, Branches: []BranchSnapshot{{Name: "A", TipSHA: "sha-A"}}}, + Worktrees: &worktree.Context{ + Origin: worktree.Location{Path: origin}, Touched: map[string]string{"inserted": "sha-created"}, + }, + CreatedBranches: map[string]string{"inserted": "sha-created"}, + } + state.RecordStack(&modified) + require.NoError(t, SaveState(dir, state)) + refs := map[string]string{"A": "sha-A", "inserted": "sha-created"} + mock := newApplyMock(dir, refs) + mock.RootDirFn = func() (string, error) { return origin, nil } + mock.CurrentBranchFn = func() (string, error) { return "A", nil } + backup := StatePath(dir) + ".before-failure" + mock.DeleteBranchFn = func(name string, _ bool) error { + delete(refs, name) + require.NoError(t, os.Rename(StatePath(dir), backup)) + require.NoError(t, os.Mkdir(StatePath(dir), 0755)) + return nil + } + restore := git.SetOps(mock) + defer restore() + cfg, _, _ := config.NewTestConfig() + defer cfg.Out.Close() + defer cfg.Err.Close() + require.Error(t, UnwindFromStateFile(cfg, dir)) + require.NoError(t, os.Remove(StatePath(dir))) + require.NoError(t, os.Rename(backup, StatePath(dir))) + saved, err := LoadState(dir) + require.NoError(t, err) + assert.Empty(t, saved.Worktrees.Touched, "ref restoration must be durable before created refs are deleted") + mock.DeleteBranchFn = func(string, bool) error { + t.Fatal("already deleted operation-created ref must not be deleted again") + return nil + } + require.NoError(t, UnwindFromStateFile(cfg, dir)) + assert.False(t, StateExists(dir)) + final, err := stack.Load(dir) + require.NoError(t, err) + assert.Equal(t, []string{"A"}, final.Stacks[0].BranchNames()) +} + +func TestContinueApply_RejectsMergedPendingBranch(t *testing.T) { + dir := t.TempDir() + s := stack.Stack{ + Trunk: stack.BranchRef{Branch: "main"}, + Branches: []stack.BranchRef{{Branch: "A", PullRequest: &stack.PullRequestRef{Number: 1, Merged: true}}}, + } + writeTestStackFile(t, dir, s) + state := &StateFile{SchemaVersion: 1, Phase: PhaseConflict, ConflictType: "rebase", ConflictBranch: "A"} + state.RecordStack(&s) + require.NoError(t, SaveState(dir, state)) + called := false + restore := git.SetOps(&git.MockOps{ + RebaseContinueFn: func(git.RebaseOpts) error { called = true; return nil }, + }) + defer restore() + cfg, _, _ := config.NewTestConfig() + defer cfg.Out.Close() + defer cfg.Err.Close() + require.ErrorContains(t, ContinueApply(cfg, dir, noopUpdateBaseSHAs), "merged or queued branch A") + assert.False(t, called) + assert.True(t, StateExists(dir)) +} + +func TestModifyState_RejectsInvalidActionProgress(t *testing.T) { + for _, next := range []int{-1, 2} { + t.Run(fmt.Sprint(next), func(t *testing.T) { + dir := t.TempDir() + state := &StateFile{ + SchemaVersion: 1, Phase: PhaseConflict, ConflictType: "actions", + Execution: []Action{{Type: "drop", Branch: "A"}}, NextAction: next, DesiredOrder: []string{"B"}, + } + require.NoError(t, SaveState(dir, state)) + _, err := LoadState(dir) + require.ErrorContains(t, err, "invalid action progress") + assert.True(t, StateExists(dir)) + }) + } +} + +func TestContinueApply_RejectsChangedPendingBase(t *testing.T) { + dir := t.TempDir() + s := stack.Stack{Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "A"}, {Branch: "B"}}} + writeTestStackFile(t, dir, s) + state := &StateFile{ + SchemaVersion: 1, Phase: PhaseConflict, ConflictType: "rebase", ConflictBranch: "B", + Snapshot: Snapshot{Branches: []BranchSnapshot{{Name: "A", TipSHA: "original-A"}, {Name: "B", TipSHA: "original-B"}}}, + Worktrees: &worktree.Context{ + Origin: worktree.Location{Path: "/tmp/fake-repo"}, Touched: map[string]string{"A": "rewritten-A"}, + Pending: "B", PendingBefore: "original-B", + }, + } + state.RecordStack(&s) + require.NoError(t, SaveState(dir, state)) + mock := newApplyMock(dir, map[string]string{"A": "external-A", "B": "original-B"}) + mock.IsRebaseInProgressFn = func() bool { return true } + continued := false + mock.RebaseContinueFn = func(git.RebaseOpts) error { continued = true; return nil } + restore := git.SetOps(mock) + defer restore() + cfg, _, _ := config.NewTestConfig() + defer cfg.Out.Close() + defer cfg.Err.Close() + require.ErrorContains(t, ContinueApply(cfg, dir, noopUpdateBaseSHAs), "A changed since") + assert.False(t, continued) + assert.True(t, StateExists(dir)) +} + +func TestApplyPlan_RejectsReorderedFoldBeforeMutation(t *testing.T) { + repo := setupDistributedModify(t, false) + before, err := os.ReadFile(filepath.Join(repo.common, "gh-stack")) + require.NoError(t, err) + restore := git.SetOps(git.ForWorktree(repo.origin)) + defer restore() + cfg, _, _ := config.NewTestConfig() + defer cfg.Out.Close() + defer cfg.Err.Close() + nodes := makeNodes(&repo.sf.Stacks[0]) + nodes = []modifyview.ModifyBranchNode{nodes[1], nodes[0], nodes[2]} + nodes[0].PendingAction = &modifyview.PendingAction{Type: modifyview.ActionFoldUp} + nodes[0].Removed = true + + result, conflict, err := ApplyPlan(cfg, repo.common, &repo.sf.Stacks[0], repo.sf, nodes, "C", noopUpdateBaseSHAs) + if err == nil { + t.Logf("unchecked mixed plan left A=%s, main=%s, C files=%q", + runModifyGit(t, repo.root, "rev-parse", "A"), + runModifyGit(t, repo.root, "rev-parse", "main"), + runModifyGit(t, repo.root, "ls-tree", "--name-only", "C")) + } + assert.ErrorContains(t, err, "cannot mix reordering") + assert.Nil(t, result) + assert.Nil(t, conflict) + for branch, sha := range repo.refs { + assert.Equal(t, sha, runModifyGit(t, repo.root, "rev-parse", branch), branch+" must not move") + assert.Equal(t, branch, runModifyGit(t, repo.owners[branch], "branch", "--show-current")) + } + assert.Equal(t, []string{"a.txt", "b.txt", "base.txt", "c.txt"}, + strings.Fields(runModifyGit(t, repo.root, "ls-tree", "--name-only", "C"))) + after, err := os.ReadFile(filepath.Join(repo.common, "gh-stack")) + require.NoError(t, err) + assert.Equal(t, before, after, "an invalid mixed plan must not change the catalog") + assert.False(t, StateExists(repo.common), "an invalid mixed plan must not leave a modify journal") +} + +func TestCompileActions_ReorderStructureExclusivity(t *testing.T) { + s := stack.Stack{ + Trunk: stack.BranchRef{Branch: "main"}, + Branches: []stack.BranchRef{{Branch: "A"}, {Branch: "B"}, {Branch: "C"}}, + } + restore := git.SetOps(&git.MockOps{}) + defer restore() + for _, action := range []modifyview.ActionType{ + modifyview.ActionDrop, modifyview.ActionFoldDown, modifyview.ActionFoldUp, + modifyview.ActionRename, modifyview.ActionInsertBelow, modifyview.ActionInsertAbove, + } { + t.Run("reject "+string(action), func(t *testing.T) { + nodes := makeNodes(&s) + nodes = []modifyview.ModifyBranchNode{nodes[1], nodes[0], nodes[2]} + switch action { + case modifyview.ActionInsertBelow, modifyview.ActionInsertAbove: + inserted := insertedModifyNode("inserted") + inserted.PendingAction.Type = action + nodes = append(nodes[:2], inserted, nodes[2]) + case modifyview.ActionRename: + nodes[0].PendingAction = &modifyview.PendingAction{Type: action, NewName: "renamed"} + default: + nodes[0].PendingAction = &modifyview.PendingAction{Type: action} + nodes[0].Removed = true + } + _, _, err := compileActions(&s, nodes) + require.ErrorContains(t, err, "cannot mix reordering") + }) + } + + t.Run("pure reorder", func(t *testing.T) { + nodes := makeNodes(&s) + nodes[0], nodes[1] = nodes[1], nodes[0] + execution, desired, err := compileActions(&s, nodes) + require.NoError(t, err) + assert.Empty(t, execution) + assert.Equal(t, []string{"B", "A", "C"}, desired) + }) + t.Run("insertion shifts positions without reordering", func(t *testing.T) { + nodes := makeNodes(&s) + for i := range nodes { + nodes[i].OriginalPosition = len(nodes) - 1 - i + } + nodes = []modifyview.ModifyBranchNode{nodes[0], insertedModifyNode("inserted"), nodes[1], nodes[2]} + _, desired, err := compileActions(&s, nodes) + require.NoError(t, err) + assert.Equal(t, []string{"A", "inserted", "B", "C"}, desired) + }) + t.Run("fold with original TUI display positions", func(t *testing.T) { + nodes := makeNodes(&s) + for i := range nodes { + nodes[i].OriginalPosition = len(nodes) - 1 - i + } + nodes[1].PendingAction = &modifyview.PendingAction{Type: modifyview.ActionFoldUp} + nodes[1].Removed = true + execution, desired, err := compileActions(&s, nodes) + require.NoError(t, err) + require.Len(t, execution, 1) + assert.Equal(t, "C", execution[0].Target) + assert.Equal(t, []string{"A", "C"}, desired) + }) +} + +func TestDistributedModify_FoldUpAcrossDropExcludesDroppedHistory(t *testing.T) { + repo := setupDistributedModify(t, false) + restore := git.SetOps(git.ForWorktree(repo.origin)) + defer restore() + cfg, _, _ := config.NewTestConfig() + defer cfg.Out.Close() + defer cfg.Err.Close() + nodes := makeNodes(&repo.sf.Stacks[0]) + nodes[1].PendingAction = &modifyview.PendingAction{Type: modifyview.ActionDrop} + nodes[1].Removed = true + nodes[0].PendingAction = &modifyview.PendingAction{Type: modifyview.ActionFoldUp} + nodes[0].Removed = true + _, conflict, err := ApplyPlan(cfg, repo.common, &repo.sf.Stacks[0], repo.sf, nodes, "C", noopUpdateBaseSHAs) + require.NoError(t, err) + assert.Nil(t, conflict) + assert.Equal(t, []string{"C"}, repo.sf.Stacks[0].BranchNames()) + assert.Equal(t, []string{"a.txt", "base.txt", "c.txt"}, + strings.Fields(runModifyGit(t, repo.root, "ls-tree", "--name-only", "C"))) + assert.Equal(t, []string{"A", "C"}, + strings.Split(runModifyGit(t, repo.root, "log", "--reverse", "--format=%s", "main..C"), "\n")) + for _, source := range []string{"A", "B"} { + assert.Equal(t, repo.refs[source], runModifyGit(t, repo.root, "rev-parse", source)) + assert.Equal(t, source, runModifyGit(t, repo.owners[source], "branch", "--show-current")) + assert.Equal(t, "", runModifyGit(t, repo.owners[source], "status", "--porcelain")) + } + assert.Equal(t, "C", runModifyGit(t, repo.origin, "branch", "--show-current")) + assert.False(t, StateExists(repo.common)) +} + +func TestDistributedModify_FoldUpDropNormalizationRecovery(t *testing.T) { + for _, outcome := range []string{"continue", "abort"} { + t.Run(outcome, func(t *testing.T) { + repo := setupDistributedModify(t, true) + invoker := git.ForWorktree(repo.owners["A"]) + restore := git.SetOps(invoker) + defer restore() + cfg, _, _ := config.NewTestConfig() + defer cfg.Out.Close() + defer cfg.Err.Close() + nodes := makeNodes(&repo.sf.Stacks[0]) + nodes[1].PendingAction = &modifyview.PendingAction{Type: modifyview.ActionDrop} + nodes[1].Removed = true + nodes[0].PendingAction = &modifyview.PendingAction{Type: modifyview.ActionFoldUp} + nodes[0].Removed = true + _, conflict, err := ApplyPlan(cfg, repo.common, &repo.sf.Stacks[0], repo.sf, nodes, "A", noopUpdateBaseSHAs) + require.Error(t, err) + require.NotNil(t, conflict) + assert.Equal(t, "C", conflict.Branch) + state, err := LoadState(repo.common) + require.NoError(t, err) + require.NotNil(t, state.PendingAction) + assert.Equal(t, "fold_rebase", state.PendingAction.Type) + assert.Equal(t, 0, state.NextAction) + assert.Equal(t, repo.refs["B"], state.OriginalRefs["C"], "do not widen the cutoff before normalization completes") + assert.True(t, worktree.SamePath(state.Worktrees.Location("C").Path, repo.origin)) + assert.True(t, invoker.ForWorktree(repo.origin).IsRebaseInProgress()) + saved, err := stack.Load(repo.common) + require.NoError(t, err) + assert.Equal(t, []string{"A", "B", "C"}, saved.Stacks[0].BranchNames()) + restoreCaller := git.SetOps(invoker.ForWorktree(repo.caller)) + defer restoreCaller() + if outcome == "abort" { + require.NoError(t, UnwindFromStateFile(cfg, repo.common)) + for branch, sha := range repo.refs { + assert.Equal(t, sha, runModifyGit(t, repo.root, "rev-parse", branch)) + } + assert.Equal(t, "A\nB\nC", runModifyGit(t, repo.root, "log", "--reverse", "--format=%s", "main..C")) + } else { + require.NoError(t, os.WriteFile(filepath.Join(repo.origin, "base.txt"), []byte("C\n"), 0644)) + runModifyGit(t, repo.origin, "add", "base.txt") + require.NoError(t, ContinueApply(cfg, repo.common, noopUpdateBaseSHAs)) + assert.Equal(t, "C", runModifyGit(t, repo.root, "show", "C:base.txt")) + assert.Equal(t, "A\nC", runModifyGit(t, repo.root, "log", "--reverse", "--format=%s", "main..C")) + saved, err = stack.Load(repo.common) + require.NoError(t, err) + assert.Equal(t, []string{"C"}, saved.Stacks[0].BranchNames()) + } + for _, source := range []string{"A", "B"} { + assert.Equal(t, repo.refs[source], runModifyGit(t, repo.root, "rev-parse", source)) + assert.Equal(t, source, runModifyGit(t, repo.owners[source], "branch", "--show-current")) + } + assert.Equal(t, "C", runModifyGit(t, repo.origin, "branch", "--show-current")) + assert.False(t, invoker.ForWorktree(repo.origin).IsRebaseInProgress()) + assert.False(t, StateExists(repo.common)) + }) + } +} + +func TestDistributedModify_FoldUpAcrossMultipleDroppedRanges(t *testing.T) { + repo := setupDistributedModify(t, false) + runModifyGit(t, repo.origin, "checkout", "-qb", "D") + commitModifyFile(t, repo.origin, "d.txt", "D\n", "D") + runModifyGit(t, repo.origin, "checkout", "-qb", "E") + commitModifyFile(t, repo.origin, "e.txt", "E\n", "E") + repo.sf.Stacks[0].Branches = append(repo.sf.Stacks[0].Branches, stack.BranchRef{Branch: "D"}, stack.BranchRef{Branch: "E"}) + require.NoError(t, stack.Save(repo.common, repo.sf)) + nodes := makeNodes(&repo.sf.Stacks[0]) + for _, index := range []int{1, 3} { + nodes[index].PendingAction = &modifyview.PendingAction{Type: modifyview.ActionDrop} + nodes[index].Removed = true + } + for _, index := range []int{0, 2} { + nodes[index].PendingAction = &modifyview.PendingAction{Type: modifyview.ActionFoldUp} + nodes[index].Removed = true + } + restore := git.SetOps(git.ForWorktree(repo.origin)) + defer restore() + execution, _, err := compileActions(&repo.sf.Stacks[0], nodes) + require.NoError(t, err) + var excluded []string + for _, action := range execution { + if action.Type == "fold_rebase" { + excluded = append(excluded, action.Target) + } + } + assert.Equal(t, []string{"D", "B"}, excluded, "normalize each dropped range once, from top to bottom") + cfg, _, _ := config.NewTestConfig() + defer cfg.Out.Close() + defer cfg.Err.Close() + _, _, err = ApplyPlan(cfg, repo.common, &repo.sf.Stacks[0], repo.sf, nodes, "E", noopUpdateBaseSHAs) + require.NoError(t, err) + assert.Equal(t, []string{"E"}, repo.sf.Stacks[0].BranchNames()) + assert.Equal(t, []string{"a.txt", "base.txt", "c.txt", "e.txt"}, + strings.Fields(runModifyGit(t, repo.root, "ls-tree", "--name-only", "E"))) + assert.Equal(t, "A\nC\nE", runModifyGit(t, repo.root, "log", "--reverse", "--format=%s", "main..E")) +} diff --git a/internal/modify/plan.go b/internal/modify/plan.go new file mode 100644 index 0000000..7c59b4b --- /dev/null +++ b/internal/modify/plan.go @@ -0,0 +1,255 @@ +package modify + +import ( + "fmt" + "slices" + + "github.com/github/gh-stack/internal/git" + "github.com/github/gh-stack/internal/stack" + "github.com/github/gh-stack/internal/tui/modifyview" +) + +func removesNode(n modifyview.ModifyBranchNode) bool { + if n.Removed { + return true + } + return n.PendingAction != nil && (n.PendingAction.Type == modifyview.ActionDrop || + n.PendingAction.Type == modifyview.ActionFoldDown || n.PendingAction.Type == modifyview.ActionFoldUp) +} + +// Resolve action targets before touching Git. The persisted execution order +// also lets continuation finish structural actions after a fold conflict. +func compileActions(s *stack.Stack, nodes []modifyview.ModifyBranchNode) ([]Action, []string, error) { + preview := *s + preview.Branches = append([]stack.BranchRef{}, s.Branches...) + renames := make(map[string]string) + seen := make(map[string]bool) + originalOrder := make([]string, 0, len(s.Branches)) + hasStructure, hasReorder := false, false + for _, n := range nodes { + if seen[n.Ref.Branch] { + return nil, nil, fmt.Errorf("branch %s appears more than once in the modify plan", n.Ref.Branch) + } + seen[n.Ref.Branch] = true + if n.IsInserted { + if n.PendingAction == nil || (n.PendingAction.Type != modifyview.ActionInsertAbove && n.PendingAction.Type != modifyview.ActionInsertBelow) || + n.PendingAction.NewName != n.Ref.Branch || removesNode(n) { + return nil, nil, fmt.Errorf("invalid insertion plan for %s", n.Ref.Branch) + } + } else { + originalOrder = append(originalOrder, n.Ref.Branch) + index := s.IndexOf(n.Ref.Branch) + if index < 0 { + return nil, nil, fmt.Errorf("branch %s is not in the stack being modified", n.Ref.Branch) + } + if s.Branches[index].IsMerged() && (n.PendingAction != nil || n.Removed) { + return nil, nil, fmt.Errorf("cannot modify merged branch %s", n.Ref.Branch) + } + if n.Removed && n.PendingAction == nil { + return nil, nil, fmt.Errorf("removed branch %s has no modify action", n.Ref.Branch) + } + } + if n.PendingAction != nil { + switch n.PendingAction.Type { + case modifyview.ActionRename, modifyview.ActionInsertAbove, modifyview.ActionInsertBelow: + hasStructure = true + name := n.PendingAction.NewName + if err := git.ValidateRefName(name); err != nil { + return nil, nil, fmt.Errorf("invalid branch name %q: %w", name, err) + } + if git.BranchExists(name) || s.Contains(name) { + return nil, nil, fmt.Errorf("branch %s already exists", name) + } + case modifyview.ActionDrop, modifyview.ActionFoldDown, modifyview.ActionFoldUp: + hasStructure = true + case modifyview.ActionMove: + hasReorder = true + default: + return nil, nil, fmt.Errorf("unknown modify action %q", n.PendingAction.Type) + } + } + } + for _, branch := range s.Branches { + if !seen[branch.Branch] { + return nil, nil, fmt.Errorf("branch %s is missing from the modify plan", branch.Branch) + } + } + // Compare original branch order, including removed nodes but excluding + // insertions. TUI OriginalPosition uses the opposite display orientation. + if hasStructure && (hasReorder || !slices.Equal(originalOrder, s.BranchNames())) { + return nil, nil, fmt.Errorf("cannot mix reordering with drops, folds, inserts, or renames in one modify session; apply them separately") + } + + var execution []Action + for _, n := range nodes { + if n.PendingAction == nil || n.PendingAction.Type != modifyview.ActionRename { + continue + } + name := n.PendingAction.NewName + if preview.Contains(name) { + return nil, nil, fmt.Errorf("branch %s is used by multiple modify actions", name) + } + renames[n.Ref.Branch] = name + preview.Branches[preview.IndexOf(n.Ref.Branch)].Branch = name + execution = append(execution, Action{Type: "rename", Branch: n.Ref.Branch, NewName: name}) + } + resolved := func(name string) string { + if renamed := renames[name]; renamed != "" { + return renamed + } + return name + } + desired := make([]string, 0, len(nodes)) + for _, n := range nodes { + if !removesNode(n) && !n.Ref.IsMerged() { + desired = append(desired, resolved(n.Ref.Branch)) + } + } + for _, n := range nodes { + if !n.IsInserted { + continue + } + name := n.PendingAction.NewName + if preview.Contains(name) { + return nil, nil, fmt.Errorf("branch %s is used by multiple modify actions", name) + } + position := slices.Index(desired, name) + if position < 0 { + return nil, nil, fmt.Errorf("inserted branch %s has no position in the stack", name) + } + parent := s.Trunk.Branch + if position > 0 { + parent = desired[position-1] + } + index := 0 + if parent != s.Trunk.Branch { + index = preview.IndexOf(parent) + 1 + if index == 0 { + return nil, nil, fmt.Errorf("insertion parent %s is missing from the stack", parent) + } + } + preview.Branches = slices.Insert(preview.Branches, index, stack.BranchRef{Branch: name}) + execution = append(execution, Action{ + Type: string(n.PendingAction.Type), Branch: name, NewName: name, Target: parent, NewPosition: index, + }) + } + normalizedDrops := make(map[string]map[string]bool) + for i, n := range nodes { + if n.PendingAction == nil || (n.PendingAction.Type != modifyview.ActionFoldDown && n.PendingAction.Type != modifyview.ActionFoldUp) { + continue + } + direction := -1 + if n.PendingAction.Type == modifyview.ActionFoldUp { + direction = 1 + } + target := "" + targetPosition := -1 + for j := i + direction; j >= 0 && j < len(nodes); j += direction { + candidate := nodes[j] + if removesNode(candidate) || candidate.Ref.IsMerged() { + continue + } + if candidate.IsInserted { + return nil, nil, fmt.Errorf("cannot fold %s into an inserted branch", n.Ref.Branch) + } + target = resolved(candidate.Ref.Branch) + targetPosition = j + break + } + if target == "" { + return nil, nil, fmt.Errorf("no surviving branch to fold %s into", n.Ref.Branch) + } + if n.PendingAction.Type == modifyview.ActionFoldUp { + if normalizedDrops[target] == nil { + normalizedDrops[target] = make(map[string]bool) + } + // Remove upper dropped ranges first so lower original cutoffs + // remain ancestors while the receiver is normalized. + for j := targetPosition - 1; j > i; j-- { + dropped := nodes[j] + if dropped.PendingAction == nil || dropped.PendingAction.Type != modifyview.ActionDrop { + continue + } + name := resolved(dropped.Ref.Branch) + if !normalizedDrops[target][name] { + execution = append(execution, Action{Type: "fold_rebase", Branch: target, Target: name}) + normalizedDrops[target][name] = true + } + } + } + execution = append(execution, Action{Type: string(n.PendingAction.Type), Branch: resolved(n.Ref.Branch), Target: target}) + } + for i := len(nodes) - 1; i >= 0; i-- { + n := nodes[i] + if n.PendingAction != nil && n.PendingAction.Type == modifyview.ActionDrop { + execution = append(execution, Action{Type: "drop", Branch: resolved(n.Ref.Branch)}) + } + } + return execution, desired, nil +} + +func applyOrder(s *stack.Stack, desired []string) error { + branches := make(map[string]stack.BranchRef) + for _, branch := range s.Branches { + if !branch.IsMerged() { + branches[branch.Branch] = branch + } + } + if len(branches) != len(desired) { + return fmt.Errorf("the modify plan no longer matches the stack's active branches") + } + next := 0 + for i, branch := range s.Branches { + if branch.IsMerged() { + continue + } + replacement, ok := branches[desired[next]] + if !ok { + return fmt.Errorf("branch %s is missing from the planned stack order", desired[next]) + } + delete(branches, desired[next]) + s.Branches[i] = replacement + next++ + } + return nil +} + +func plannedRef(state *StateFile, name string) string { + for _, action := range state.Plan { + if action.Type == "rename" && action.NewName == name && state.RenamedBranches[action.Branch] == "" { + return action.Branch + } + } + if renamed := state.RenamedBranches[name]; renamed != "" { + return renamed + } + return name +} + +func remainingTargets(state *StateFile) []string { + var branches []string + if state.DesiredOrder != nil && state.NextAction < len(state.Execution) { + for _, action := range state.Execution[state.NextAction:] { + switch action.Type { + case "rename", "fold_rebase": + branches = append(branches, action.Branch) + case "fold_down": + branches = append(branches, action.Target) + } + } + branches = append(branches, state.DesiredOrder...) + } else if state.ConflictType == "" && state.DesiredOrder != nil { + branches = append(branches, state.DesiredOrder...) + } else { + branches = append(branches, state.RemainingBranches...) + if state.ConflictType == "rebase_start" || state.ConflictType == "rebase" || state.ConflictType == "" { + if state.ConflictBranch != "" { + branches = append(branches, state.ConflictBranch) + } + } + } + for i, name := range branches { + branches[i] = plannedRef(state, name) + } + return branches +} diff --git a/internal/modify/preconditions.go b/internal/modify/preconditions.go index 6d1ddc2..61aec80 100644 --- a/internal/modify/preconditions.go +++ b/internal/modify/preconditions.go @@ -9,28 +9,129 @@ import ( "github.com/github/gh-stack/internal/worktree" ) -// CheckWorktrees permits modify in a linked worktree, but not a stack whose -// member branches are checked out in other worktrees. Trunk is only read. +// CheckWorktrees resolves the initiating worktree. Action-specific owner +// preflight runs after the TUI has produced its plan. func CheckWorktrees(s *stack.Stack) (*worktree.Context, error) { ctx, err := worktree.New() if err != nil { return nil, err } - if err := checkSingleWorktree(ctx, s.BranchNames()); err != nil { + ops, err := originOps(ctx) + if err != nil { + return nil, err + } + if err := worktree.CheckClean(ops, ctx.Origin.Path); err != nil { return nil, err } + return ctx, nil +} + +func originOps(ctx *worktree.Context) (git.Ops, error) { ops, err := ctx.OriginOps() if err != nil { return nil, err } - if err := worktree.CheckClean(ops, ctx.Origin.Path); err != nil { + if err := validateWorktreeRoot(ops, ctx.Origin.Path); err != nil { return nil, err } - return ctx, nil + return ops, nil +} + +func validateWorktreeRoot(ops git.Ops, path string) error { + root, err := ops.RootDir() + if err != nil { + return fmt.Errorf("resolving the working-tree root at %q: %w", path, err) + } + if !worktree.SamePath(root, path) { + return fmt.Errorf("%q is not a working-tree root; restore or repair the worktree before continuing", path) + } + return nil +} + +func branchOps(ctx *worktree.Context, branch string) (git.Ops, error) { + ops, err := ctx.Ops(branch) + if err != nil { + return nil, err + } + if err := validateWorktreeRoot(ops, ctx.Location(branch).Path); err != nil { + return nil, err + } + return ops, nil +} + +func validateOwner(ctx *worktree.Context, branch string, trees []git.Worktree) error { + location := ctx.Location(branch) + owner := "" + for _, tree := range trees { + if tree.Bare || tree.Branch != branch { + continue + } + if owner != "" && !worktree.SamePath(owner, tree.Path) { + return fmt.Errorf("branch %s is checked out in multiple worktrees", branch) + } + owner = tree.Path + if tree.Prunable { + return fmt.Errorf("worktree holding %s is unavailable at %s; repair it before continuing", branch, tree.Path) + } + } + if owner != "" && !worktree.SamePath(owner, location.Path) { + return fmt.Errorf("branch %s changed worktree owners since modify began (%s instead of %s); leaving it untouched", branch, owner, location.Path) + } + if owner == "" && !worktree.SamePath(location.Path, ctx.Origin.Path) { + return fmt.Errorf("branch %s is no longer checked out in its recorded worktree %s", branch, location.Path) + } + return nil +} + +func preflightBranches(state *StateFile, branches []string, pending string) error { + ctx := state.Worktrees + pendingPath := "" + if pending != "" { + if _, err := branchOps(ctx, pending); err != nil { + return err + } + pendingPath = ctx.Location(pending).Path + } + trees, err := git.Worktrees() + if err != nil { + return fmt.Errorf("checking modify worktree ownership: %w", err) + } + if pending != "" { + if err := validateOwner(ctx, pending, trees); err != nil { + return err + } + } + checked := make(map[string]bool) + for _, branch := range branches { + if checked[branch] { + continue + } + checked[branch] = true + if _, err := branchOps(ctx, branch); err != nil { + return err + } + // A native conflict intentionally leaves this worktree busy. Its + // remaining branches are checked immediately before their actions. + if pendingPath != "" && worktree.SamePath(ctx.Location(branch).Path, pendingPath) { + continue + } + if err := validateOwner(ctx, branch, trees); err != nil { + return err + } + if err := ctx.Preflight([]string{branch}); err != nil { + return err + } + if err := checkExpectedRef(state, branch); err != nil { + return err + } + } + return nil } +// Journals without an origin cannot safely acquire distributed owners during +// recovery. Keep their original-worktree compatibility path conservative. func checkSingleWorktree(ctx *worktree.Context, branches []string) error { - if _, err := ctx.OriginOps(); err != nil { + if _, err := originOps(ctx); err != nil { return err } trees, err := git.Worktrees() @@ -43,7 +144,7 @@ func checkSingleWorktree(ctx *worktree.Context, branches []string) error { } for _, tree := range trees { if tree.Branch != "" && members[tree.Branch] && !worktree.SamePath(tree.Path, ctx.Origin.Path) { - return fmt.Errorf("distributed modify is not supported yet: branch %s is checked out in worktree %s; all stack branches must be unoccupied or in %s", tree.Branch, tree.Path, ctx.Origin.Path) + return fmt.Errorf("legacy modify recovery cannot acquire another worktree for %s; return its branches to the original worktree first", tree.Branch) } } return nil diff --git a/internal/modify/recovery.go b/internal/modify/recovery.go index 5419054..443b3bb 100644 --- a/internal/modify/recovery.go +++ b/internal/modify/recovery.go @@ -60,8 +60,11 @@ func recoveryContext(dir string, state *StateFile) (*worktree.Context, error) { if !worktree.SamePath(nativeDir, dir) { return nil, fmt.Errorf("legacy modify recovery must run in its original worktree before shared-state migration") } + if err := checkSingleWorktree(ctx, recoveryBranches(state)); err != nil { + return nil, err + } } - if err := checkSingleWorktree(ctx, recoveryBranches(state)); err != nil { + if _, err := originOps(ctx); err != nil { return nil, err } return ctx, nil @@ -171,30 +174,71 @@ func originalTips(state *StateFile) map[string]string { return refs } -func prepareMutation(state *StateFile) (git.Ops, error) { - if err := checkSingleWorktree(state.Worktrees, recoveryBranches(state)); err != nil { - return nil, err +func expectedHead(state *StateFile, branch string) string { + if sha := state.Worktrees.Touched[branch]; sha != "" { + return sha + } + if sha := state.CreatedBranches[branch]; sha != "" { + return sha + } + for _, original := range state.Snapshot.Branches { + name := original.Name + if renamed := state.RenamedBranches[name]; renamed != "" { + name = renamed + } + if name == branch { + return original.TipSHA + } } - ops, err := state.Worktrees.OriginOps() + if branch == state.StackName { + return state.TrunkSHA + } + return "" +} + +func checkExpectedRef(state *StateFile, branch string) error { + expected := expectedHead(state, branch) + if expected == "" { + return nil // Legacy records and not-yet-created insertion targets. + } + current, err := git.RevParse(branch) if err != nil { - return nil, err + return fmt.Errorf("reading expected head for %s: %w", branch, err) } - if err := worktree.CheckClean(ops, state.Worktrees.Origin.Path); err != nil { + if current != expected { + return fmt.Errorf("%s changed since this operation's snapshot; leaving it untouched", branch) + } + return nil +} + +func checkResultRefs(state *StateFile, s *stack.Stack) error { + for _, branch := range s.Branches { + if !branch.IsMerged() { + if err := checkExpectedRef(state, branch.Branch); err != nil { + return err + } + } + } + return checkExpectedRef(state, s.Trunk.Branch) +} + +func prepareMutation(state *StateFile, branch string) (git.Ops, error) { + if err := preflightBranches(state, []string{branch}, ""); err != nil { return nil, err } - return ops, nil + return branchOps(state.Worktrees, branch) } func startRefMutation(dir string, state *StateFile, branch string) (git.Ops, error) { - ops, err := prepareMutation(state) + ops, err := prepareMutation(state, branch) if err != nil { return nil, err } - expected := state.Worktrees.Touched[branch] - if expected == "" { - expected = originalTips(state)[branch] - } + expected := expectedHead(state, branch) if expected == "" { + if state.DesiredOrder != nil { + return nil, fmt.Errorf("no original head recorded for %s; recovery state was retained", branch) + } err = state.Worktrees.Start(branch) } else { err = state.Worktrees.Start(branch, expected) @@ -202,16 +246,71 @@ func startRefMutation(dir string, state *StateFile, branch string) (git.Ops, err if err != nil { return nil, err } + state.PendingHead = state.Worktrees.PendingBefore if err := SaveState(dir, state); err != nil { return nil, err } return ops, nil } +func nativeBranch(state *StateFile) string { + if state.Worktrees != nil && state.Worktrees.Pending != "" { + return state.Worktrees.Pending + } + if state.ConflictType == "cherry_pick" { + return state.FoldTarget + } + return state.ConflictBranch +} + +// ConflictOps resolves the native operation's worktree, which can differ from +// both the invoking worktree and the source branch of a fold. +func ConflictOps(state *StateFile) (git.Ops, string, error) { + if state == nil || state.Worktrees == nil || nativeBranch(state) == "" { + return nil, "", fmt.Errorf("modify conflict has no recorded worktree") + } + branch := nativeBranch(state) + ops, err := branchOps(state.Worktrees, branch) + if err != nil { + return nil, "", err + } + return ops, state.Worktrees.Location(branch).Path, nil +} + +func checkPendingHead(state *StateFile, ops git.Ops) error { + expected := state.PendingHead + if expected == "" { + expected = state.Worktrees.PendingBefore + } + if expected == "" { + return nil + } + branch := nativeBranch(state) + sha, err := ops.RevParse(branch) + if err != nil { + return err + } + if sha != expected { + return fmt.Errorf("%s changed after modify paused; leaving it untouched in %s", branch, state.Worktrees.Location(branch).Path) + } + return nil +} + +func saveNativeConflict(dir string, state *StateFile, s *stack.Stack, sf *stack.StackFile, ops git.Ops) error { + sha, err := ops.RevParse(nativeBranch(state)) + if err != nil { + return fmt.Errorf("recording paused modify head: %w", err) + } + state.PendingHead = sha + state.Phase = PhaseConflict + return saveProgress(dir, state, s, sf) +} + func finishApply(dir string, state *StateFile, s *stack.Stack, sf *stack.StackFile, needsSubmit bool) error { state.Phase = PhaseApplying state.ConflictBranch, state.ConflictType = "", "" state.RemainingBranches = nil + state.PendingHead = "" if err := saveProgress(dir, state, s, sf); err != nil { return err } @@ -224,6 +323,177 @@ func finishApply(dir string, state *StateFile, s *stack.Stack, sf *stack.StackFi return ClearState(dir) } +func foreignCheckoutOwner(ctx *worktree.Context, branch string) (string, error) { + trees, err := git.Worktrees() + if err != nil { + return "", err + } + owner := "" + for _, tree := range trees { + if tree.Bare || tree.Branch != branch { + continue + } + if owner != "" && !worktree.SamePath(owner, tree.Path) { + return "", fmt.Errorf("branch %s is checked out in multiple worktrees", branch) + } + if tree.Prunable { + return "", fmt.Errorf("worktree holding %s is unavailable at %s", branch, tree.Path) + } + if err := validateWorktreeRoot(git.ForWorktree(tree.Path), tree.Path); err != nil { + return "", err + } + owner = tree.Path + } + if owner != "" && !worktree.SamePath(owner, ctx.Origin.Path) { + return owner, nil + } + return "", nil +} + +func restoreCheckout(cfg *config.Config, state *StateFile, s *stack.Stack, aborting bool) error { + ctx := state.Worktrees + if _, err := originOps(ctx); err != nil { + return err + } + original := state.OriginalBranch + if renamed := state.RenamedBranches[original]; renamed != "" { + original = renamed + } + if original == "" { + return nil + } + target := original + if !aborting { + target = resolveCheckoutBranch(state.OriginalBranch, state.Plan, state.Snapshot, s) + } + owner, err := foreignCheckoutOwner(ctx, target) + if err != nil { + return err + } + if owner != "" { + if aborting { + return fmt.Errorf("cannot restore original checkout %s in %s: it is now owned by %s; recovery state was retained", target, ctx.Origin.Path, owner) + } + if originalOwner, err := foreignCheckoutOwner(ctx, original); err != nil { + return err + } else if originalOwner != "" { + return fmt.Errorf("cannot retain original branch %s in %s: it is now owned by %s", original, ctx.Origin.Path, originalOwner) + } + if err := ctx.RestoreOrigin(original); err != nil { + return err + } + cfg.Infof("Kept %s in %s; surviving branch %s is checked out in worktree %s", original, ctx.Origin.Path, target, owner) + return nil + } + if err := ctx.RestoreOrigin(target); err != nil { + return err + } + if target != original { + cfg.Printf("Switched to %s in %s (original branch %s is no longer in the stack)", target, ctx.Origin.Path, state.OriginalBranch) + } + return nil +} + +func abortNative(state *StateFile) error { + ctx := state.Worktrees + branch := nativeBranch(state) + if branch == "" { + return nil + } + ops, err := branchOps(ctx, branch) + if err != nil { + return err + } + trees, err := git.Worktrees() + if err != nil { + return err + } + if err := validateOwner(ctx, branch, trees); err != nil { + return err + } + rebasing, picking := ops.IsRebaseInProgress(), ops.IsCherryPickInProgress() + if !rebasing && !picking && ctx.Pending != "" { + sha, err := ops.RevParse(branch) + if err != nil { + return err + } + if sha == ctx.PendingBefore { + ctx.Pending, ctx.PendingBefore = "", "" + state.PendingHead, state.PendingAction = "", nil + return nil + } + } + if err := checkPendingHead(state, ops); err != nil { + return err + } + path := ctx.Location(branch).Path + if rebasing { + if state.ConflictType != "rebase" && state.ConflictType != "rebase_start" { + return fmt.Errorf("an unrelated rebase is active in %s; recovery state was retained", path) + } + if err := ops.RebaseAbort(); err != nil { + return fmt.Errorf("aborting rebase in %s: %w", path, err) + } + } + if picking { + if state.ConflictType != "cherry_pick" { + return fmt.Errorf("an unrelated cherry-pick is active in %s; recovery state was retained", path) + } + if err := ops.CherryPickAbort(); err != nil { + return fmt.Errorf("aborting cherry-pick in %s: %w", path, err) + } + } + if ctx.Pending != "" { + sha, err := ops.RevParse(branch) + if err != nil { + return err + } + if sha == ctx.PendingBefore { + ctx.Pending, ctx.PendingBefore = "", "" + } else if !rebasing && !picking && state.PendingHead != "" && sha == state.PendingHead { + if err := ctx.Record(branch); err != nil { + return err + } + } else { + return fmt.Errorf("cannot prove the native operation on %s was restored safely; recovery state was retained", branch) + } + } + state.PendingHead, state.PendingAction = "", nil + state.ConflictBranch, state.ConflictType = "", "" + return nil +} + +func preflightRestore(state *StateFile) error { + original := originalTips(state) + trees, err := git.Worktrees() + if err != nil { + return err + } + for branch, expected := range state.Worktrees.Touched { + ops, err := branchOps(state.Worktrees, branch) + if err != nil { + return err + } + sha, err := ops.RevParse(branch) + if err != nil { + return err + } + if sha == original[branch] { + continue // A previous attempt restored it before a journal save failed. + } + if sha != expected { + return fmt.Errorf("%s changed after this operation; leaving it untouched", branch) + } + if err := validateOwner(state.Worktrees, branch, trees); err != nil { + return err + } + if err := worktree.CheckClean(ops, state.Worktrees.Location(branch).Path); err != nil { + return err + } + } + return nil +} + func unwindState(cfg *config.Config, dir string, state *StateFile, sf *stack.StackFile) error { if state.Phase != PhaseApplying && state.Phase != PhaseConflict { return fmt.Errorf("cannot unwind modify in phase %q; recovery state was retained", state.Phase) @@ -243,7 +513,7 @@ func unwindState(cfg *config.Config, dir string, state *StateFile, sf *stack.Sta if err != nil { return err } - ops, err := ctx.OriginOps() + ops, err := originOps(ctx) if err != nil { return err } @@ -260,31 +530,24 @@ func unwindState(cfg *config.Config, dir string, state *StateFile, sf *stack.Sta return errors.Join(err, SaveState(dir, state)) } - if ops.IsRebaseInProgress() { - if state.Worktrees != nil && state.ConflictType != "rebase" && state.ConflictType != "rebase_start" { - return retain(fmt.Errorf("an unrelated rebase is active in %s; recovery state was retained", ctx.Origin.Path)) - } - if err := ops.RebaseAbort(); err != nil { - return retain(fmt.Errorf("aborting rebase in %s: %w", ctx.Origin.Path, err)) - } - } - if ops.IsCherryPickInProgress() { - if state.Worktrees != nil && state.ConflictType != "cherry_pick" { - return retain(fmt.Errorf("an unrelated cherry-pick is active in %s; recovery state was retained", ctx.Origin.Path)) - } - if err := ops.CherryPickAbort(); err != nil { - return retain(fmt.Errorf("aborting cherry-pick in %s: %w", ctx.Origin.Path, err)) - } - } - if err := worktree.CheckClean(ops, ctx.Origin.Path); err != nil { - return retain(err) - } - originalBranch := state.OriginalBranch if originalBranch == "" && len(state.Snapshot.Branches) > 0 { originalBranch = state.Snapshot.Branches[0].Name } if state.Worktrees == nil { + if ops.IsRebaseInProgress() { + if err := ops.RebaseAbort(); err != nil { + return retain(err) + } + } + if ops.IsCherryPickInProgress() { + if err := ops.CherryPickAbort(); err != nil { + return retain(err) + } + } + if err := worktree.CheckClean(ops, ctx.Origin.Path); err != nil { + return retain(err) + } if err := restoreLegacy(ops, state, originalBranch); err != nil { return retain(err) } @@ -292,12 +555,22 @@ func unwindState(cfg *config.Config, dir string, state *StateFile, sf *stack.Sta if err := recoverPendingAction(state, ops); err != nil { return retain(err) } + if err := abortNative(state); err != nil { + return retain(err) + } + if err := SaveState(dir, state); err != nil { + return err + } for i := len(state.Plan) - 1; i >= 0; i-- { action := state.Plan[i] newName, renamed := state.RenamedBranches[action.Branch] if action.Type != "rename" || !renamed { continue } + ops, err := branchOps(ctx, newName) + if err != nil { + return retain(err) + } if ops.BranchExists(newName) { sha, err := ops.RevParse(newName) if err != nil { @@ -306,6 +579,11 @@ func unwindState(cfg *config.Config, dir string, state *StateFile, sf *stack.Sta if sha != ctx.Touched[newName] || ops.BranchExists(action.Branch) { return retain(fmt.Errorf("renamed branch %s changed after modify; leaving it untouched", newName)) } + state.PendingAction = &Action{Type: "undo_rename", Branch: newName, NewName: action.Branch} + ops, err = startRefMutation(dir, state, newName) + if err != nil { + return retain(err) + } if err := ops.RenameBranch(newName, action.Branch); err != nil { return retain(fmt.Errorf("restoring branch name %s: %w", action.Branch, err)) } @@ -316,30 +594,53 @@ func unwindState(cfg *config.Config, dir string, state *StateFile, sf *stack.Sta } } ctx.Rename(newName, action.Branch) + if err := ctx.Record(action.Branch); err != nil { + return retain(err) + } delete(state.RenamedBranches, action.Branch) + state.PendingAction, state.PendingHead = nil, "" if err := SaveState(dir, state); err != nil { return err } } + if err := preflightRestore(state); err != nil { + return retain(err) + } if err := ctx.Restore(originalTips(state)); err != nil { return retain(err) } - if originalBranch != "" { - if err := ctx.RestoreOrigin(originalBranch); err != nil { - return retain(err) - } + if err := SaveState(dir, state); err != nil { + return err + } + state.OriginalBranch = originalBranch + if err := restoreCheckout(cfg, state, &restored, true); err != nil { + return retain(err) } for name, original := range state.CreatedBranches { + ops, err := branchOps(ctx, name) + if err != nil { + return retain(err) + } if ops.BranchExists(name) { sha, err := ops.RevParse(name) if err != nil || sha != original { return retain(fmt.Errorf("inserted branch %s changed after modify; leaving it untouched", name)) } + trees, err := git.Worktrees() + if err != nil { + return retain(err) + } + if err := validateOwner(ctx, name, trees); err != nil { + return retain(err) + } if err := ops.DeleteBranch(name, true); err != nil { return retain(fmt.Errorf("removing inserted branch %s: %w", name, err)) } } delete(state.CreatedBranches, name) + if err := SaveState(dir, state); err != nil { + return err + } } } @@ -360,20 +661,37 @@ func recoverPendingAction(state *StateFile, ops git.Ops) error { return nil } switch action.Type { - case "rename": + case "rename", "undo_rename": + var err error + ops, err = branchOps(state.Worktrees, action.Branch) + if err != nil { + return err + } if ops.BranchExists(action.NewName) && !ops.BranchExists(action.Branch) { sha, err := ops.RevParse(action.NewName) if err != nil || sha != state.Worktrees.PendingBefore { return fmt.Errorf("cannot identify interrupted rename to %s; recovery state was retained", action.NewName) } - if state.RenamedBranches == nil { - state.RenamedBranches = make(map[string]string) + if action.Type == "rename" { + if state.RenamedBranches == nil { + state.RenamedBranches = make(map[string]string) + } + state.RenamedBranches[action.Branch] = action.NewName + } else { + delete(state.RenamedBranches, action.NewName) } - state.RenamedBranches[action.Branch] = action.NewName state.Worktrees.Rename(action.Branch, action.NewName) + trees, err := git.Worktrees() + if err != nil { + return err + } + if err := validateOwner(state.Worktrees, action.NewName, trees); err != nil { + return err + } if err := state.Worktrees.Record(action.NewName); err != nil { return err } + state.PendingHead = "" } else if !ops.BranchExists(action.Branch) || ops.BranchExists(action.NewName) { return fmt.Errorf("cannot identify interrupted rename of %s; recovery state was retained", action.Branch) } @@ -381,6 +699,10 @@ func recoverPendingAction(state *StateFile, ops git.Ops) error { if ops.BranchExists(action.NewName) && state.CreatedBranches[action.NewName] == "" { return fmt.Errorf("cannot prove branch %s was created by the interrupted insert; recovery state was retained", action.NewName) } + case "fold_down", "fold_rebase": + return nil // The native operation must be aborted in its receiver. + default: + return fmt.Errorf("unknown pending modify action %q; recovery state was retained", action.Type) } state.PendingAction = nil return nil diff --git a/internal/modify/state.go b/internal/modify/state.go index 9975a8a..6dd32c2 100644 --- a/internal/modify/state.go +++ b/internal/modify/state.go @@ -38,6 +38,11 @@ type StateFile struct { RenamedBranches map[string]string `json:"renamed_branches,omitempty"` CreatedBranches map[string]string `json:"created_branches,omitempty"` PendingAction *Action `json:"pending_action,omitempty"` + Execution []Action `json:"execution,omitempty"` + NextAction int `json:"next_action,omitempty"` + DesiredOrder []string `json:"desired_order"` + TrunkSHA string `json:"trunk_sha,omitempty"` + PendingHead string `json:"pending_head,omitempty"` // Conflict state — populated when phase is "conflict" ConflictBranch string `json:"conflict_branch,omitempty"` @@ -75,6 +80,7 @@ type Action struct { Branch string `json:"branch"` NewPosition int `json:"new_position,omitempty"` // for "move" NewName string `json:"new_name,omitempty"` // for "rename" + Target string `json:"target,omitempty"` // fold receiver, insertion parent, or normalization drop } // StatePath returns the full path to the modify state file. @@ -100,6 +106,9 @@ func LoadState(gitDir string) (*StateFile, error) { if state.SchemaVersion > 1 { return nil, fmt.Errorf("modify state uses unsupported schema version %d; upgrade gh-stack before recovery", state.SchemaVersion) } + if state.DesiredOrder != nil && (state.NextAction < 0 || state.NextAction > len(state.Execution)) { + return nil, fmt.Errorf("modify state has invalid action progress; recovery state was retained") + } return &state, nil } diff --git a/skills/gh-stack/SKILL.md b/skills/gh-stack/SKILL.md index 34f77e8..3fd1866 100644 --- a/skills/gh-stack/SKILL.md +++ b/skills/gh-stack/SKILL.md @@ -172,9 +172,10 @@ an ancestor of the branch. create/remove worktrees. Mutations serialize across the clone, and paused operations require recovery in their recorded owners. Legacy recovery must finish in its original worktree before migration. -- Core `modify` temporarily rejects distributed stack branches before TUI/apply. Linked-worktree - use is allowed when all member branches are unoccupied or owned here; trunk ownership alone is - not a blocker. Its recovery flags still use the recorded origin from any linked worktree. +- `modify` supports distributed stack branches, but its editor remains TUI-only. Actions run in + affected clean owners; unoccupied branches use the origin. Drop/fold source branches and + worktrees are preserved. Recovery flags may run from any worktree and use recorded native + operation owners; never resolve/stage in the caller's tree unless the diagnostic names it. - There is no non-interactive reorder or removal. Errors may suggest `gh stack modify`, but it is TUI-only — restructure with `unstack` then `init` instead. - PR titles and bodies are auto-generated. Use `gh pr edit` afterwards to change them. diff --git a/skills/gh-stack/references/troubleshooting.md b/skills/gh-stack/references/troubleshooting.md index f622e0c..3bd45fb 100644 --- a/skills/gh-stack/references/troubleshooting.md +++ b/skills/gh-stack/references/troubleshooting.md @@ -151,11 +151,12 @@ target, check the exit status, and change directory to the quoted output. Only a owners are updated by rebase/sync; commit or stash manually when those owners are dirty. gh-stack does not automatically stash or create/remove worktrees. -For `git init --separate-git-dir` repositories, Git may list the administration directory as the -main path instead of the actual checkout. Operations from a known main or linked origin remain -supported, but main-owner discovery from another checkout can be unavailable. Do not navigate to -an administration directory or guess its associated checkout; run from the actual main worktree -when its working files are needed. No private registry or Git config changes are used to infer it. +For `git init --separate-git-dir` repositories, main invocation and existing absolute/relative +`core.worktree` backlinks are supported, including settings in the main `config.worktree`. The +discovery caveat is only linked invocation without a main-worktree backlink. A required unresolved +main owner produces actionable guidance to run from the main worktree or supply the backlink; +unaffected worktrees continue. Never navigate to an administration directory or guess its checkout. +Existing backlinks are read without adding a private registry or changing Git configuration. ## Stack file is locked (exit 8) @@ -176,8 +177,10 @@ gh stack modify --abort Related: `submit` also detects a pending modify state, and under a TTY asks before overwriting the matching stack on GitHub with local state. An unrelated stack cannot consume or clear that journal. -Core modify temporarily rejects stack branches checked out in other worktrees. It works in a -linked worktree when every member branch is unoccupied or owned there; a foreign trunk is allowed. -Shared-journal continue/abort executes in the recorded origin even when invoked elsewhere. Native -Git markers remain per-worktree. If recovery reports missing owners, externally changed refs, or -save failures, fix the reported problem and retry; the journal is retained to prevent false success. +Modify supports stacks distributed across worktrees, but agents must still not launch its TUI. +Renames, fold-down cherry-picks, and rebases execute in the appropriate clean owners; only the +origin switches for unoccupied branches. Drop/fold sources and their worktrees remain intact. +Resolve and stage in the worktree reported by the conflict, then invoke `modify --continue` from +any linked worktree. A later conflict can be in a different owner. Abort reverses owner-local +renames, restores only operation-touched refs, and deletes only proven operation-created refs, +never worktrees. Missing owners, externally changed refs, or save failures retain the journal.