diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 5c17103..df27e03 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -37,8 +37,8 @@ No Makefile, no code generation, no external linter config. Standard Go toolchai - Load stack files with `stack.Load(dir)` after writing to get correct checksums. - 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. Context-tracked modify passes the snapshot SHA (or prior `Context.Touched` SHA) to `Context.Start`; never claim an external commit as this operation's work during continuation. -- Rebase/sync currently reject foreign-owned members and writable trunks after prerequisite migration but before requested mutations, including sync reconciliation. Their existing engine remains origin-only; rebase recovery must be invoked in its recorded origin. +- Finish paused operations before switching preview stages or versions. Reject origin-only and unknown rebase execution modes before routing recovery; use the matching layer3 build in the recorded origin for origin-only journals. +- 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. For full architecture details, see [AGENTS.md](../AGENTS.md) in the repository root. diff --git a/AGENTS.md b/AGENTS.md index af05959..66cebfa 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -124,12 +124,11 @@ if errors.As(err, &exitErr) { ... } - **Locking:** `/gh-stack.lock` protects short catalog saves; `/gh-stack-operation.lock` serializes clone-wide mutations. Acquire `beginStackMutation` before snapshots/preflight and defer its cleanup. Never hold a catalog lock across Git operations or call lock-taking `stack.Save` while already holding that lock. Errors surface as `LockError`. - **Staleness:** Concurrent modifications detected via `StaleError`. - **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 the origin, stack identity, original refs, and progress. Native Git markers remain per-worktree. Rebase continue/abort currently requires the recorded original worktree; modify uses a scoped origin executor even when invoked elsewhere. 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. Context-tracked modify calls `Context.Start(branch, expectedSHA)` before ref mutations, using the snapshot or last `Context.Touched` SHA. Do not adopt a freshly read tip as the operation's baseline during continuation. +- **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. -- **Intermediate rebase/sync boundary:** Keep the existing origin-only execution engine. After prerequisite catalog migration, reject all foreign-owned member/rollback targets and any trunk that would be updated before requested mutations. Sync must check remote-added/replacement branches before importing or saving membership. Stack selection cannot eagerly checkout before this preflight. Multi-owner execution is deferred. -- **Journal transition:** New rebase/sync journals carry `executionMode: "origin-only"`. Their context identifies the origin, not distributed per-step progress. Future engines must recognize this marker before routing recovery and either use compatible origin-bound recovery or fail closed with matching-build instructions. Never reinterpret it as a distributed journal; this build likewise rejects unmarked non-null contexts and unknown modes. Legacy null-context journals retain their original-catalog route. +- **Journal transition:** Complete paused operations before switching preview stages or versions. Unmarked rebase/sync journals with a worktree context use distributed recovery. Reject `executionMode: "origin-only"` before context-based routing or mutation; preserve its bytes and require the matching layer3 build in the recorded origin. Unknown nonempty modes also fail closed. Empty-mode journals without a context retain the legacy original-catalog route. ## CI workflows (`.github/workflows/`) @@ -145,5 +144,6 @@ if errors.As(err, &exitErr) { ... } - `git.SetOps()` replaces the **package-level** ops variable. Forgetting `defer restore()` in a test will break every subsequent test in the package. - Interrupt detection: Ctrl+C is caught as `terminal.InterruptErr`, wrapped into an `errInterrupt` sentinel, and printed with a friendly message before a silent exit. - Rerere: on first rebase conflict, the user is prompted to enable `git rerere`. If declined, a flag file prevents future prompts. `tryAutoResolveRebase()` loops up to 1000 times auto-continuing when rerere resolves conflicts. +- Rebase commands disable automatic maintenance through command-local configuration so detached `rerere gc` cannot race conflict handling. Repository settings and unrelated Git commands are unchanged. - Date-preserving rebase starts use the merge backend so Git persists the date setting across conflicts. Continuations use native saved settings, not start-only date flags. - The `.gitignore` ignores `/gh-stack` and `/gh-stack.exe` (the built binary). diff --git a/README.md b/README.md index f406757..f46fa8d 100644 --- a/README.md +++ b/README.md @@ -64,13 +64,15 @@ Stack metadata is stored in `/gh-stack` (a JSON file, not committed The gh-stack recovery journals, `gh-stack-rebase-state` and `gh-stack-modify-state`, also live in the common directory and record the worktrees involved. Git's own HEAD, index, rebase, and cherry-pick markers remain **per worktree**. -On upgrade, nonconflicting legacy worktree catalogs are consolidated automatically and originals are preserved as backups. Migration is a prerequisite and can complete even if the requested rewrite is subsequently refused. Conflicting definitions stop migration rather than choosing one; the error identifies the files to reconcile. Finish or abort legacy in-progress operations in their original worktree first. Do not mix old and new gh-stack versions within one clone. +On upgrade, nonconflicting legacy worktree catalogs are consolidated automatically and originals are preserved as backups. Conflicting definitions stop migration rather than choosing one; the error identifies the files to reconcile. Finish or abort legacy in-progress operations in their original worktree first. Do not mix old and new gh-stack versions within one clone. + +Complete paused operations before switching preview stages or gh-stack versions. If recovery reports an incompatible execution lifecycle, finish or abort it using the matching build in its recorded original worktree; do not edit or remove the journal. ### Git worktrees -You can keep independent stacks in linked worktrees or track a stack whose branches are distributed across them. **For now, `rebase` and `sync` require all stack branches to be unoccupied or checked out in the initiating worktree.** They conservatively refuse foreign-owned members, even outside a requested rebase range, before changing refs, checkouts, stack membership, or remote stacks. A foreign-owned trunk is also refused when trunk updates are enabled; `rebase --no-trunk` does not update it. +You can keep independent stacks in linked worktrees or distribute a stack's branches across them. `rebase` and `sync` automatically update the clean worktree that owns each affected branch. Dirty, busy, missing, or changed owners stop the operation; unrelated worktrees are left alone. A trunk that cannot safely fast-forward can use the existing fetched-remote fallback. -Mutations are serialized across the clone, while read-only views remain available. Paused operations must be continued or aborted before another mutation. Rebase recovery must be invoked in its recorded original worktree; invoking it elsewhere fails without changing either checkout. Modify recovery can be invoked elsewhere and still executes in its recorded origin. gh-stack never automatically stashes changes, creates/removes worktrees, or steals another checkout. +Mutations are serialized across the clone, while read-only views remain available. Paused operations must be continued or aborted before another mutation. Recovery runs in the recorded worktree even when `--continue` or `--abort` is invoked elsewhere. gh-stack never automatically stashes changes, creates/removes worktrees, or steals another checkout. Mutation locks coordinate **gh-stack processes only**, not arbitrary Git commands, editors, or other tools. Keep affected worktrees idle while history is being rewritten. During a pause, make only the requested conflict-resolution edits and staging in the reported worktree. diff --git a/cmd/rebase.go b/cmd/rebase.go index 62cedd7..d743348 100644 --- a/cmd/rebase.go +++ b/cmd/rebase.go @@ -6,9 +6,9 @@ import ( "fmt" "os" "path/filepath" - "slices" "strings" + cligit "github.com/cli/cli/v2/git" "github.com/github/gh-stack/internal/config" "github.com/github/gh-stack/internal/git" "github.com/github/gh-stack/internal/modify" @@ -36,6 +36,9 @@ type rebaseState struct { StackTrunk string `json:"stackTrunk,omitempty"` StackBranches []string `json:"stackBranches,omitempty"` OriginalStack *stack.Stack `json:"originalStack,omitempty"` + RebaseBase string `json:"rebaseBase,omitempty"` + RebaseOldBase string `json:"rebaseOldBase,omitempty"` + RebaseOnto bool `json:"rebaseOnto,omitempty"` CurrentBranchIndex int `json:"currentBranchIndex"` ConflictBranch string `json:"conflictBranch"` RemainingBranches []string `json:"remainingBranches"` @@ -69,12 +72,7 @@ layer in its commit history, rebasing if necessary. Use --no-trunk to skip fetching and rebasing with the trunk branch. Only the inter-branch rebases are performed (branch 2 onto branch 1, -branch 3 onto branch 2, etc.). - -All stack branches and any trunk being updated must currently be unoccupied -or checked out in this worktree. Cross-worktree rewrites are refused before -requested mutations, after shared-catalog migration. Continue and abort must -be run in the worktree where the rebase started.`, +branch 3 onto branch 2, etc.).`, Example: ` # Rebase the entire stack $ gh stack rebase @@ -127,7 +125,8 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error { defer release() gitDir, err := stackStateDir(cfg) if err != nil { - return err + cfg.Errorf("not a git repository") + return ErrNotInStack } if opts.cont { @@ -151,40 +150,41 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error { s := result.Stack currentBranch := result.CurrentBranch originalTrunk := s.Trunk.Branch - if err := requireLocalBranches(cfg, s.BranchNames()); err != nil { - return err - } - ctx, err := worktree.New() - if err != nil { - cfg.Errorf("%s", err) - return ErrSilent - } + anchor := currentBranch if opts.branch != "" { anchor = opts.branch } - var remote string - if !opts.noTrunk { - remote, err = pickRemote(cfg, anchor, opts.remote) - if err != nil { - if !errors.Is(err, errInterrupt) { - cfg.Errorf("%s", err) - } - return ErrSilent - } - trunkBranch, err := normalizeTrunkBranch(s.Trunk.Branch, remote) - if err != nil { - cfg.Errorf("%s", err) - return ErrSilent - } - if err := requireLocalBranches(cfg, []string{trunkBranch}); err != nil { - return err - } + currentIdx := s.IndexOf(anchor) + if currentIdx < 0 { + currentIdx = 0 + } + startIdx, endIdx := 0, len(s.Branches) + if endIdx == 0 { + cfg.Printf("No branches to rebase") + return nil } - if err := worktree.CheckClean(git.CurrentOps(), ctx.Origin.Path); err != nil { + if opts.downstack { + endIdx = currentIdx + 1 + } + if opts.upstack { + startIdx = currentIdx + } + if opts.noTrunk && startIdx < 1 { + startIdx = 1 + } + branchesToRebase := s.Branches[startIdx:endIdx] + if len(branchesToRebase) == 0 { + cfg.Printf("No branches to rebase") + return nil + } + _ = syncStackPRs(cfg, s) + ctx, err := worktree.New() + if err != nil { cfg.Errorf("%s", err) return ErrSilent } + required := rebaseBranchNames(branchesToRebase) // Enable git rerere so conflict resolutions are remembered. if err := ensureRerere(cfg); errors.Is(err, errInterrupt) { @@ -193,9 +193,13 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error { var trunk trunkTarget if !opts.noTrunk { - trunk, err = resolveTrunkTarget(cfg, s, remote, currentBranch) + // Resolve remote for fetch and trunk comparison + remote, err := pickRemote(cfg, anchor, opts.remote) if err != nil { - return err + if !errors.Is(err, errInterrupt) { + cfg.Errorf("%s", err) + } + return ErrSilent } // Fast-forward stack branches that are behind their remote tracking branch. @@ -203,52 +207,36 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error { cfg.Errorf("failed to fetch stack branches from %s: %v", remote, err) return ErrSilent } - fastForwardBranches(cfg, s, remote, currentBranch) + planned := planFastForwardBranches(s, remote) + for _, forward := range planned { + required = append(required, forward.Branch) + } + if err := ctx.Preflight(required); err != nil { + cfg.Errorf("%s", err) + return ErrSilent + } + trunk, err = resolveTrunkTarget(cfg, s, remote, currentBranch, trunkResolveOptions{Worktrees: ctx}) + if err != nil { + return err + } + if _, err := fastForwardBranches(cfg, planned, ctx); err != nil { + cfg.Errorf("%s", err) + return ErrSilent + } + } else if err := ctx.Preflight(required); err != nil { + cfg.Errorf("%s", err) + return ErrSilent } cfg.Printf("Stack detected: %s", s.DisplayChain()) - currentIdx := s.IndexOf(anchor) - if currentIdx < 0 { - currentIdx = 0 - } - if len(s.Branches) == 0 { - cfg.Printf("No branches to rebase") - return nil - } - if opts.upstack && currentIdx >= 0 && s.Branches[currentIdx].IsMerged() { cfg.Warningf("Current branch %q has already been merged", currentBranch) } - startIdx := 0 - endIdx := len(s.Branches) - - if opts.downstack { - endIdx = currentIdx + 1 - } - if opts.upstack { - startIdx = currentIdx - } - - // With --no-trunk, skip the first branch (which would rebase onto trunk). - if opts.noTrunk && startIdx < 1 { - startIdx = 1 - } - - branchesToRebase := s.Branches[startIdx:endIdx] - - if len(branchesToRebase) == 0 { - cfg.Printf("No branches to rebase") - return nil - } - cfg.Printf("Rebasing branches in order, starting from %s to %s", branchesToRebase[0].Branch, branchesToRebase[len(branchesToRebase)-1].Branch) - // Sync PR state before rebase so we can detect merged PRs. - _ = syncStackPRs(cfg, s) - originalRefs, err := resolveOriginalRefs(s) if err != nil { return fmt.Errorf("resolving branch refs: %w", err) @@ -292,12 +280,16 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error { OntoOldBase: ontoOldBase, CommitterDateIsAuthorDate: opts.committerDateIsAuthorDate, TrunkRef: trunk.Ref, + TrunkSHA: trunk.SHA, + Worktrees: ctx, + State: state, + StateDir: gitDir, }) if rebaseResult.Err != nil { cfg.Errorf("%v", rebaseResult.Err) - if err := abortRebase(cfg, gitDir); err != nil { - return err + if err := rollbackWorktreeRebase(cfg, gitDir, state); err != nil { + return errors.Join(ErrSilent, err) } return ErrSilent } @@ -305,36 +297,17 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error { if rebaseResult.Conflicted { cfg.Warningf("Rebasing %s onto %s — conflict", rebaseResult.ConflictBranch, rebaseResult.ConflictBase) - state.Phase = "conflict" - state.CurrentBranchIndex = rebaseResult.ConflictIdx - state.ConflictBranch = rebaseResult.ConflictBranch - state.RemainingBranches = rebaseResult.Remaining - state.UseOnto, state.OntoOldBase = rebaseResult.NeedsOnto, rebaseResult.OntoOldBase - if err := saveRebaseState(gitDir, state); err != nil { - cfg.Errorf("failed to save conflict progress; run `gh stack rebase --abort`: %s", err) - return ErrSilent - } - - printConflictDetails(cfg, rebaseResult.ConflictBase) + printWorktreeConflict(cfg, state, rebaseResult.ConflictBase) cfg.Printf("") cfg.Printf("Resolve conflicts on %s, then run `%s`", rebaseResult.ConflictBranch, cfg.ColorCyan("gh stack rebase --continue")) cfg.Printf("Or abort this operation with `%s`", cfg.ColorCyan("gh stack rebase --abort")) - cfg.Printf("Run recovery in the original worktree: %s", ctx.Origin.Path) return ErrConflict } - if unstacked := verifyStacked(s, trunk.Ref, startIdx, endIdx); len(unstacked) > 0 { - reportUnstacked(cfg, trunk.Ref, unstacked) - if err := abortRebase(cfg, gitDir); err != nil { - return err - } - return ErrSilent - } - - if err := finishOriginRebase(cfg, gitDir, state, sf, s); err != nil { + if err := finishWorktreeRebase(cfg, gitDir, state, sf, s); err != nil { return err } @@ -375,12 +348,11 @@ func continueRebase(cfg *config.Config, gitDir string) error { } return ErrSilent } - if err := requireRebaseOrigin(cfg, gitDir, state); err != nil { - return err + if state.Worktrees != nil { + return continueWorktreeRebase(cfg, gitDir, state) } - if state.Phase != "" && state.Phase != "conflict" && state.Phase != "complete" { - cfg.Errorf("rebase stopped in phase %q; run `gh stack rebase --abort` in the original worktree", state.Phase) - return ErrSilent + if err := validateLegacyRebasePhase(state, true); err != nil { + return err } sf, err := stack.Load(gitDir) @@ -391,12 +363,7 @@ func continueRebase(cfg *config.Config, gitDir string) error { // Use the saved original branch to find the stack, since git may be in // a detached HEAD state during an active rebase. - var s *stack.Stack - if state.Worktrees != nil { - s, err = rebaseStackFromState(sf, state) - } else { - s, err = resolveStack(sf, state.OriginalBranch, cfg) - } + s, err := resolveStack(sf, state.OriginalBranch, cfg) if err != nil { return err } @@ -404,7 +371,7 @@ func continueRebase(cfg *config.Config, gitDir string) error { return fmt.Errorf("no stack found for branch %s", state.OriginalBranch) } if state.Phase == "complete" { - return finishOriginRebase(cfg, gitDir, state, sf, s) + return finishLegacyRebase(cfg, gitDir, state, sf, s) } trunkRef := state.TrunkRef if trunkRef == "" { @@ -494,7 +461,7 @@ func continueRebase(cfg *config.Config, gitDir string) error { if result.Err != nil { cfg.Errorf("%v", result.Err) - if err := abortRebase(cfg, gitDir); err != nil { + if err := rollbackLegacyRebase(cfg, gitDir, state); err != nil { return err } return ErrSilent @@ -510,7 +477,8 @@ func continueRebase(cfg *config.Config, gitDir string) error { state.UseOnto = result.NeedsOnto state.OntoOldBase = result.OntoOldBase if err := saveRebaseState(gitDir, state); err != nil { - cfg.Warningf("failed to save rebase state: %s", err) + cfg.Errorf("failed to save rebase state: %s", err) + return errors.Join(ErrSilent, err) } printConflictDetails(cfg, result.ConflictBase) @@ -532,27 +500,13 @@ func continueRebase(cfg *config.Config, gitDir string) error { } if unstacked := verifyStacked(s, trunkBase, verifyStart, verifyEnd); len(unstacked) > 0 { reportUnstacked(cfg, trunkRef, unstacked) - if err := abortRebase(cfg, gitDir); err != nil { + if err := rollbackLegacyRebase(cfg, gitDir, state); err != nil { return err } return ErrSilent } - if err := finishOriginRebase(cfg, gitDir, state, sf, s); err != nil { - return err - } - - if state.NoTrunk { - cfg.Printf("All branches in stack rebased locally (without trunk)") - } else if state.TrunkSHA != "" { - cfg.Printf("All branches in stack rebased locally with %s (%s)", trunkRef, short(state.TrunkSHA)) - } else { - cfg.Printf("All branches in stack rebased locally with %s", trunkRef) - } - cfg.Printf("To push up your changes and open/update the stack of PRs, run `%s`", - cfg.ColorCyan("gh stack submit")) - - return nil + return finishLegacyRebase(cfg, gitDir, state, sf, s) } func abortRebase(cfg *config.Config, gitDir string) error { @@ -565,229 +519,137 @@ func abortRebase(cfg *config.Config, gitDir string) error { } return ErrSilent } - if err := requireRebaseOrigin(cfg, gitDir, state); err != nil { + if state.Worktrees != nil { + if err := rollbackWorktreeRebase(cfg, gitDir, state); err != nil { + return errors.Join(ErrSilent, err) + } + cfg.Successf("Rebase aborted and branches restored") + return nil + } + if err := validateLegacyRebasePhase(state, false); err != nil { return err } - if state.OriginalStack != nil { - sf, err := stack.Load(gitDir) - if err != nil { - return err - } - if _, err := rebaseStackFromState(sf, state); err != nil { - return err + if err := rollbackLegacyRebase(cfg, gitDir, state); err != nil { + return err + } + cfg.Successf("Rebase aborted and branches restored") + return nil +} + +func validateLegacyRebasePhase(state *rebaseState, cont bool) error { + switch state.Phase { + case "", "conflict", "complete": + return nil + case "applying", "restoring": + if !cont { + return nil } + return fmt.Errorf("legacy rebase is in phase %q; run `gh stack rebase --abort` to restore the stack", state.Phase) + default: + return fmt.Errorf("unknown legacy rebase phase %q; recovery state was retained", state.Phase) } +} + +func rollbackLegacyRebase(cfg *config.Config, gitDir string, state *rebaseState) error { inProgress, err := git.IsRebaseInProgress() if err != nil { return fmt.Errorf("checking rebase state: %w", err) } for branch := range state.OriginalRefs { - if _, err := git.BranchExists(branch); err != nil { + exists, err := git.BranchExists(branch) + if err != nil { return fmt.Errorf("checking branch %s before restoring: %w", branch, err) } + if exists { + if _, err := git.RevParse(branch); err != nil { + return fmt.Errorf("reading branch %s before restoring: %w", branch, err) + } + } + } + root, err := git.RootDir() + if err != nil { + return fmt.Errorf("finding recovery worktree: %w", err) + } + if !inProgress || state.Phase == "complete" { + if err := worktree.CheckClean(git.CurrentOps(), root); err != nil { + return err + } } state.Phase = "restoring" if err := saveRebaseState(gitDir, state); err != nil { - cfg.Errorf("saving recovery state before restoration: %s", err) - return ErrSilent + return err } if inProgress { if err := git.RebaseAbort(); err != nil { cfg.Errorf("aborting rebase; recovery state was retained: %s", err) - return ErrSilent + return errors.Join(ErrSilent, err) } } - root, err := git.RootDir() - if err != nil { - cfg.Errorf("finding recovery worktree: %s", err) - return ErrSilent - } if err := worktree.CheckClean(git.CurrentOps(), root); err != nil { cfg.Errorf("recovery state was retained: %s", err) - return ErrSilent + return errors.Join(ErrSilent, err) } - var restoreErrors []string - for branch, sha := range state.OriginalRefs { - if current, err := git.RevParse(branch); err == nil && current == sha { - continue - } - if err := git.CheckoutBranch(branch); err != nil { - restoreErrors = append(restoreErrors, fmt.Sprintf("checkout %s: %s", branch, err)) - continue - } - if err := git.ResetHard(sha); err != nil { - restoreErrors = append(restoreErrors, fmt.Sprintf("reset %s: %s", branch, err)) - } - } - if err := git.CheckoutBranch(state.OriginalBranch); err != nil { - restoreErrors = append(restoreErrors, fmt.Sprintf("restoring original checkout: %s", err)) - } - if len(restoreErrors) > 0 { - cfg.Warningf("Some branches could not be fully restored; recovery state was retained:") - for _, e := range restoreErrors { - cfg.Printf(" %s", e) - } - return ErrSilent - } - if state.OriginalStack != nil { - if err := restoreWorktreeRebaseMetadata(gitDir, state); err != nil { - cfg.Errorf("restoring metadata; recovery state was retained: %s", err) - return ErrSilent - } - } - if err := clearRebaseState(gitDir); err != nil { - cfg.Errorf("branches restored, but recovery state could not be cleared: %s", err) - return ErrSilent + if err := restoreRebaseRefs(cfg, state.OriginalBranch, state.OriginalRefs); err != nil { + return err } - - cfg.Successf("Rebase aborted and branches restored") - return nil + return clearRebaseState(gitDir) } -func newWorktreeRebaseState(s *stack.Stack, ctx *worktree.Context, originalBranch string, refs map[string]string, trunk trunkTarget, start, end int) *rebaseState { - snapshot := *s - snapshot.Branches = append([]stack.BranchRef{}, s.Branches...) - return &rebaseState{ - ExecutionMode: originOnlyRebaseMode, - Phase: "applying", - Worktrees: ctx, - StackID: s.ID, - StackTrunk: s.Trunk.Branch, - StackBranches: s.BranchNames(), - OriginalStack: &snapshot, - OriginalBranch: originalBranch, - OriginalRefs: refs, - CurrentBranchIndex: start, - StartIndex: start, - EndIndex: end, - TrunkRef: trunk.Ref, - TrunkSHA: trunk.SHA, +func restoreRebaseCheckout(branch string) error { + current, err := git.CurrentBranch() + if err != nil && !errors.Is(err, cligit.ErrNotOnAnyBranch) { + return fmt.Errorf("reading original checkout: %w", err) } -} - -func requireRebaseOrigin(cfg *config.Config, dir string, state *rebaseState) error { - localDir, err := git.GitDir() + if err == nil && current == branch { + return nil + } + root, err := git.RootDir() if err != nil { return err } - if state.Worktrees == nil { - if !worktree.SamePath(localDir, dir) { - cfg.Errorf("legacy rebase recovery must run in its original worktree (Git directory %s)", dir) - return ErrRebaseActive - } - } else { - ops, err := state.Worktrees.OriginOps() - if err != nil { - cfg.Errorf("%s", err) - return ErrRebaseActive - } - originDir, err := ops.GitDir() - if err != nil { - return err - } - if !worktree.SamePath(localDir, originDir) { - cfg.Errorf("rebase recovery must run in its original worktree: %s", state.Worktrees.Origin.Path) - cfg.Printf("Return there before running `gh stack rebase --continue` or `gh stack rebase --abort`") - return ErrRebaseActive - } - } - branches := append([]string{state.OriginalBranch, state.ConflictBranch}, state.RemainingBranches...) - for name := range state.OriginalRefs { - branches = append(branches, name) - } - if err := requireLocalBranches(cfg, branches); err != nil { + if err := worktree.CheckClean(git.CurrentOps(), root); err != nil { return err } - if state.Phase == "complete" { - root, err := git.RootDir() - if err != nil { - return err - } - if err := worktree.CheckClean(git.CurrentOps(), root); err != nil { - cfg.Errorf("recovery state was retained: %s", err) - return ErrSilent - } + if err := git.CheckoutBranch(branch); err != nil { + return fmt.Errorf("restoring original checkout %s: %w", branch, err) } return nil } -func rebaseStackFromState(sf *stack.StackFile, state *rebaseState) (*stack.Stack, error) { - switch state.Phase { - case "applying", "conflict", "complete", "restoring": - default: - return nil, fmt.Errorf("unknown saved rebase phase %q", state.Phase) - } - var target *stack.Stack - for i := range sf.Stacks { - s := &sf.Stacks[i] - if s.Trunk.Branch != state.StackTrunk || !slices.Equal(s.BranchNames(), state.StackBranches) { - continue - } - if (state.Phase == "applying" || state.Phase == "conflict") && state.StackID != "" && s.ID != "" && state.StackID != s.ID { - continue - } - if target != nil { - return nil, fmt.Errorf("saved rebase matches multiple stacks; resolve the catalog before continuing") +func finishLegacyRebase(cfg *config.Config, gitDir string, state *rebaseState, sf *stack.StackFile, s *stack.Stack) error { + if state.Phase != "complete" { + state.Phase = "complete" + state.RemainingBranches = nil + if err := saveRebaseState(gitDir, state); err != nil { + return err } - target = s - } - if target == nil { - return nil, fmt.Errorf("the stack changed since this rebase started; restore its original membership or run gh stack rebase --abort") - } - if state.StartIndex < 0 || state.EndIndex > len(target.Branches) || - state.StartIndex > state.EndIndex || state.CurrentBranchIndex < state.StartIndex || - state.CurrentBranchIndex > state.EndIndex { - return nil, fmt.Errorf("invalid branch range in saved rebase state") - } - return target, nil -} - -func restoreWorktreeRebaseMetadata(dir string, state *rebaseState) error { - sf, err := stack.Load(dir) - if err != nil { - return err } - target, err := rebaseStackFromState(sf, state) - if err != nil { + if err := restoreRebaseCheckout(state.OriginalBranch); err != nil { return err } - if !slices.Equal(state.OriginalStack.BranchNames(), target.BranchNames()) { - return fmt.Errorf("original stack snapshot does not match the recovery target") - } - target.Trunk.Head = state.OriginalStack.Trunk.Head - for i, before := range state.OriginalStack.Branches { - target.Branches[i].Base = before.Base - target.Branches[i].Head = before.Head - if sha, err := git.RevParse(before.Branch); err == nil { - target.Branches[i].Head = sha - } else if !before.IsMerged() { - return fmt.Errorf("reading restored branch %s: %w", before.Branch, err) - } - } - return stack.Save(dir, sf) -} - -func finishOriginRebase(cfg *config.Config, dir string, state *rebaseState, sf *stack.StackFile, s *stack.Stack) error { - state.Phase = "complete" - state.RemainingBranches = nil - if err := saveRebaseState(dir, state); err != nil { - cfg.Errorf("saving completed rebase; recovery state was retained: %s", err) - return ErrSilent - } - if err := git.CheckoutBranch(state.OriginalBranch); err != nil { - cfg.Errorf("restoring original checkout; recovery state was retained: %s", err) - return ErrSilent - } updateBaseSHAsWithTrunk(s, state.TrunkSHA) _ = syncStackPRs(cfg, s) - if err := stack.Save(dir, sf); err != nil { + if err := stack.Save(gitDir, sf); err != nil { return handleSaveError(cfg, err) } - if err := clearRebaseState(dir); err != nil { - cfg.Errorf("rebase completed but recovery state could not be cleared: %s", err) - return ErrSilent + if err := clearRebaseState(gitDir); err != nil { + return fmt.Errorf("rebase completed but recovery state could not be cleared: %w", err) + } + + trunkRef := state.TrunkRef + if trunkRef == "" { + trunkRef = s.Trunk.Branch } + if state.NoTrunk { + cfg.Printf("All branches in stack rebased locally (without trunk)") + } else if state.TrunkSHA != "" { + cfg.Printf("All branches in stack rebased locally with %s (%s)", trunkRef, short(state.TrunkSHA)) + } else { + cfg.Printf("All branches in stack rebased locally with %s", trunkRef) + } + cfg.Printf("To push up your changes and open/update the stack of PRs, run `%s`", + cfg.ColorCyan("gh stack submit")) return nil } @@ -811,23 +673,24 @@ func loadRebaseState(gitDir string) (*rebaseState, error) { if err := json.Unmarshal(data, &state); err != nil { return nil, err } + if err := validateRebaseExecutionMode(&state); err != nil { + return nil, err + } + return &state, nil +} + +func validateRebaseExecutionMode(state *rebaseState) error { + if state.ExecutionMode == "" { + return nil + } origin := "its original worktree" if state.Worktrees != nil && state.Worktrees.Origin.Path != "" { origin = fmt.Sprintf("worktree %q", state.Worktrees.Origin.Path) } - switch state.ExecutionMode { - case originOnlyRebaseMode: - if state.Worktrees == nil { - return nil, fmt.Errorf("origin-only rebase journal has no recorded worktree; recovery state was retained") - } - case "": - if state.Worktrees != nil { - return nil, fmt.Errorf("this rebase journal uses a different execution lifecycle; use the matching gh-stack build in %s to continue or abort", origin) - } - default: - return nil, fmt.Errorf("unsupported rebase execution mode %q; use the matching gh-stack build in %s to continue or abort", state.ExecutionMode, origin) + if state.ExecutionMode == originOnlyRebaseMode { + return fmt.Errorf("origin-only rebase journal requires the matching layer3 gh-stack build in %s to continue or abort; recovery state was retained", origin) } - return &state, nil + return fmt.Errorf("unsupported rebase execution mode %q; use the matching gh-stack build in %s to continue or abort; recovery state was retained", state.ExecutionMode, origin) } func clearRebaseState(gitDir string) error { diff --git a/cmd/rebase_test.go b/cmd/rebase_test.go index 15a3e9c..8feda3c 100644 --- a/cmd/rebase_test.go +++ b/cmd/rebase_test.go @@ -76,6 +76,66 @@ func TestRebase_RecoveryStateLookupFailurePreservesJournal(t *testing.T) { } } +func TestRebase_WorktreeRecoveryLookupFailurePreservesJournal(t *testing.T) { + for _, action := range []string{"continue", "abort"} { + for _, query := range []string{"factory", "native state", "branch"} { + if action == "continue" && query == "branch" { + continue + } + t.Run(action+"/"+query, func(t *testing.T) { + dir := t.TempDir() + s := stack.Stack{ + Trunk: stack.BranchRef{Branch: "main"}, + Branches: []stack.BranchRef{{Branch: "b1"}, {Branch: "b2"}}, + } + writeStackFile(t, dir, s) + mock := newRebaseMock(dir, "b1") + restore := git.SetOps(mock) + defer restore() + ctx, err := worktree.New() + require.NoError(t, err) + ctx.Pending, ctx.PendingBefore = "b1", "old-b1" + ctx.Touched["b2"] = "new-b2" + state := newWorktreeRebaseState(&s, ctx, "b1", map[string]string{"b1": "old-b1", "b2": "old-b2"}, trunkTarget{}, 0, 2) + state.Phase, state.ConflictBranch = "conflict", "b1" + require.NoError(t, saveRebaseState(dir, state)) + beforeJournal, err := os.ReadFile(filepath.Join(dir, rebaseStateFile)) + require.NoError(t, err) + beforeCatalog, err := os.ReadFile(filepath.Join(dir, "gh-stack")) + require.NoError(t, err) + lookupErr := fmt.Errorf("%s lookup failed", query) + switch query { + case "factory": + mock.ForWorktreeFn = func(string) (git.Ops, error) { return nil, lookupErr } + case "native state": + mock.IsRebaseInProgressFn = func() (bool, error) { return true, lookupErr } + case "branch": + mock.BranchExistsFn = func(branch string) (bool, error) { + if branch == "b2" { + return false, lookupErr + } + return true, nil + } + } + forbidRewriteMutations(t, mock) + mock.RebaseContinueFn = func(git.RebaseOpts) error { t.Fatal("must not continue after a lookup error"); return nil } + mock.RebaseAbortFn = func() error { t.Fatal("must not abort after a lookup error"); return nil } + cfg := issue250TestConfig(t) + + err = runRebase(cfg, &rebaseOptions{cont: action == "continue", abort: action == "abort"}) + + require.ErrorIs(t, err, lookupErr) + afterJournal, err := os.ReadFile(filepath.Join(dir, rebaseStateFile)) + require.NoError(t, err) + assert.Equal(t, beforeJournal, afterJournal) + afterCatalog, err := os.ReadFile(filepath.Join(dir, "gh-stack")) + require.NoError(t, err) + assert.Equal(t, beforeCatalog, afterCatalog) + }) + } + } +} + // rebaseCall records arguments passed to RebaseOnto or Rebase. type rebaseCall struct { newBase string @@ -160,7 +220,7 @@ func TestRebase_CascadeRebase(t *testing.T) { // All branches should be rebased in order: b1 onto main, b2 onto b1, b3 onto b2 require.Len(t, allRebaseCalls, 3) - assert.Equal(t, "main", allRebaseCalls[0].newBase, "b1 should be rebased onto trunk") + assert.Equal(t, "sha-main", allRebaseCalls[0].newBase, "b1 should be rebased onto the pinned trunk") assert.Equal(t, "b1", allRebaseCalls[1].newBase, "b2 should be rebased onto b1") assert.Equal(t, "b2", allRebaseCalls[2].newBase, "b3 should be rebased onto b2") @@ -225,7 +285,7 @@ func TestRebase_MergedBranch_UsesOnto(t *testing.T) { // b2: onto trunk, oldBase = b1's original SHA // b3: onto b2, oldBase = b2's original SHA (propagation) require.Len(t, rebaseCalls, 2) - assert.Equal(t, rebaseCall{"main", "b1-orig-sha", "b2"}, rebaseCalls[0], + assert.Equal(t, rebaseCall{"default-sha", "b1-orig-sha", "b2"}, rebaseCalls[0], "b2 should rebase --onto main using b1's original SHA as oldBase") assert.Equal(t, rebaseCall{"b2", "b2-orig-sha", "b3"}, rebaseCalls[1], "b3 should propagate --onto mode with b2's original SHA as oldBase") @@ -295,7 +355,7 @@ func TestRebase_OntoPropagatesToSubsequentBranches(t *testing.T) { // b4: first non-merged ancestor = b3 → newBase = b3 // RebaseOnto("b3", "b3-orig-sha", "b4") require.Len(t, rebaseCalls, 2) - assert.Equal(t, rebaseCall{"main", "b2-orig-sha", "b3"}, rebaseCalls[0], + assert.Equal(t, rebaseCall{"default-sha", "b2-orig-sha", "b3"}, rebaseCalls[0], "b3 should rebase --onto main with b2's SHA as oldBase") assert.Equal(t, rebaseCall{"b3", "b3-orig-sha", "b4"}, rebaseCalls[1], "b4 should rebase --onto b3 with b3's original SHA as oldBase") @@ -372,7 +432,7 @@ func TestRebase_StaleOntoOldBase_UsesForkPoint(t *testing.T) { require.Len(t, rebaseCalls, 2) // b2: stale ontoOldBase detected → uses fork-point(main, b2) - assert.Equal(t, rebaseCall{"main", "main-b2-forkpoint", "b2"}, rebaseCalls[0], + assert.Equal(t, rebaseCall{"default-sha", "main-b2-forkpoint", "b2"}, rebaseCalls[0], "b2 should use the reflog fork-point when ontoOldBase is stale") // b3: b2's SHA is a valid ancestor → uses it directly @@ -486,6 +546,8 @@ func TestRebase_Abort_RestoresBranches(t *testing.T) { currentBranch := "b2" // simulating we're on the conflict branch mock := newRebaseMock(tmpDir, currentBranch) + mock.BranchExistsFn = func(string) (bool, error) { return true, nil } + mock.CurrentBranchFn = func() (string, error) { return currentBranch, nil } mock.CheckoutBranchFn = func(name string) error { checkouts = append(checkouts, name) currentBranch = name @@ -579,7 +641,7 @@ func TestRebase_DownstackOnly(t *testing.T) { assert.NoError(t, err) // b2 is at index 1, so downstack = [b1, b2] (indices 0..1) require.Len(t, allRebaseCalls, 2, "downstack should rebase b1 and b2 only") - assert.Equal(t, "main", allRebaseCalls[0].newBase, "b1 should be rebased onto trunk") + assert.Equal(t, "sha-main", allRebaseCalls[0].newBase, "b1 should be rebased onto the pinned trunk") assert.Equal(t, "b1", allRebaseCalls[1].newBase, "b2 should be rebased onto b1") } @@ -687,7 +749,7 @@ func TestRebase_UpstackWithMergedBranchBelow(t *testing.T) { require.Len(t, allRebaseCalls, 2, "upstack should rebase b2 and b3") // b2: --onto rebase with b1's old SHA as old base - assert.Equal(t, "main", allRebaseCalls[0].newBase, "b2 should be rebased onto main (first non-merged ancestor)") + assert.Equal(t, "sha-main", allRebaseCalls[0].newBase, "b2 should be rebased onto the pinned main (first non-merged ancestor)") assert.Equal(t, "sha-b1", allRebaseCalls[0].oldBase, "b2 should use b1's original SHA as old base") assert.Equal(t, "b2", allRebaseCalls[0].branch, "b2 should be the branch being rebased") @@ -1073,7 +1135,7 @@ func TestRebase_Continue_RebasesRemainingBranches(t *testing.T) { var checkouts []string mock := newRebaseMock(tmpDir, "b2") - mock.IsRebaseInProgressFn = func() (bool, error) { return true, nil } + mock.IsRebaseInProgressFn = func() (bool, error) { return !rebaseContinueCalled, nil } mock.RebaseContinueFn = func(opts git.RebaseOpts) error { rebaseContinueCalled = true return nil @@ -1150,8 +1212,9 @@ func TestRebase_Continue_QueuedBranchBelowConflict(t *testing.T) { mock := newRebaseMock(tmpDir, "b1") mock.BranchExistsFn = func(name string) (bool, error) { return true, nil } - mock.IsRebaseInProgressFn = func() (bool, error) { return true, nil } - mock.RebaseContinueFn = func(opts git.RebaseOpts) error { return nil } + inProgress := true + mock.IsRebaseInProgressFn = func() (bool, error) { return inProgress, nil } + mock.RebaseContinueFn = func(opts git.RebaseOpts) error { inProgress = false; return nil } mock.RebaseOntoFn = func(newBase, oldBase, branch string, opts git.RebaseOpts) error { rebaseCalls = append(rebaseCalls, rebaseCall{newBase, oldBase, branch}) return nil @@ -1223,7 +1286,7 @@ func TestRebase_Continue_OntoMode(t *testing.T) { var rebaseContinueCalled bool mock := newRebaseMock(tmpDir, "b3") - mock.IsRebaseInProgressFn = func() (bool, error) { return true, nil } + mock.IsRebaseInProgressFn = func() (bool, error) { return !rebaseContinueCalled, nil } mock.RebaseContinueFn = func(opts git.RebaseOpts) error { rebaseContinueCalled = true return nil @@ -1346,6 +1409,8 @@ func TestRebase_Abort_WithActiveRebase(t *testing.T) { currentBranch := "b2" mock := newRebaseMock(tmpDir, currentBranch) + mock.BranchExistsFn = func(string) (bool, error) { return true, nil } + mock.CurrentBranchFn = func() (string, error) { return currentBranch, nil } mock.IsRebaseInProgressFn = func() (bool, error) { return inProgress, nil } mock.RebaseAbortFn = func() error { rebaseAbortCalled = true @@ -1638,7 +1703,7 @@ func TestRebase_SkipsMergedBranchesNotExistingLocally(t *testing.T) { // Head SHA as oldBase so `git rebase --onto` receives valid arguments. require.Len(t, rebaseCalls, 1) assert.Equal(t, "b2", rebaseCalls[0].branch) - assert.Equal(t, "main", rebaseCalls[0].newBase) + assert.Equal(t, "sha-main", rebaseCalls[0].newBase) assert.Equal(t, "b1-stored-head-sha", rebaseCalls[0].oldBase) } @@ -2397,139 +2462,6 @@ func TestIntegration_AdoptedBranchRebasesFromCommonAncestor(t *testing.T) { require.NoError(t, issue250GitMayFail(t, cloneDir, "merge-base", "--is-ancestor", "parent", "imported")) } -func mockForeignOwner(t *testing.T, mock *git.MockOps, common, current, branch string) string { - t.Helper() - root, owner := t.TempDir(), t.TempDir() - mock.RootDirFn = func() (string, error) { return root, nil } - mock.CommonDirFn = func() (string, error) { return common, nil } - mock.WorktreesFn = func() ([]git.Worktree, error) { - return []git.Worktree{{Path: root, Branch: current}, {Path: owner, Branch: branch}}, nil - } - mock.ForWorktreeFn = func(path string) (git.Ops, error) { - if worktree.SamePath(path, root) { - return mock, nil - } - require.True(t, worktree.SamePath(path, owner)) - return &git.MockOps{ - CommonDirFn: func() (string, error) { return common, nil }, - GitDirFn: func() (string, error) { return filepath.Join(common, "worktrees", "owner"), nil }, - }, nil - } - return owner -} - -func forbidRewriteMutations(t *testing.T, mock *git.MockOps) { - t.Helper() - mock.CheckoutBranchFn = func(string) error { t.Fatal("unexpected checkout"); return nil } - mock.CreateBranchFn = func(string, string) error { t.Fatal("unexpected branch creation"); return nil } - mock.UpdateBranchRefFn = func(string, string) error { t.Fatal("unexpected ref update"); return nil } - mock.ResetHardFn = func(string) error { t.Fatal("unexpected reset"); return nil } - mock.MergeFFFn = func(string) error { t.Fatal("unexpected fast-forward"); return nil } - mock.RebaseFn = func(string, git.RebaseOpts) error { t.Fatal("unexpected rebase"); return nil } - mock.RebaseOntoFn = func(string, string, string, git.RebaseOpts) error { - t.Fatal("unexpected rebase onto") - return nil - } - mock.PushFn = func(string, []string, bool, bool) error { t.Fatal("unexpected push"); return nil } - mock.DeleteBranchFn = func(string, bool) error { t.Fatal("unexpected branch deletion"); return nil } - mock.DeleteTrackingRefFn = func(string, string) error { t.Fatal("unexpected tracking ref deletion"); return nil } -} - -func TestRebase_ForeignTargetsRefusedBeforeMutation(t *testing.T) { - for _, tc := range []struct { - name, owner, trunk string - opts rebaseOptions - merged bool - }{ - {name: "whole stack", owner: "b1"}, - {name: "fast-forward outside downstack range", owner: "b3", opts: rebaseOptions{downstack: true}}, - {name: "rollback outside upstack range", owner: "b1", opts: rebaseOptions{upstack: true, noTrunk: true}}, - {name: "merged rollback target", owner: "b1", merged: true}, - {name: "foreign trunk", owner: "main"}, - {name: "normalized foreign trunk", owner: "main", trunk: "origin/main"}, - } { - t.Run(tc.name, func(t *testing.T) { - dir := t.TempDir() - trunk := tc.trunk - if trunk == "" { - trunk = "main" - } - s := stack.Stack{ - Trunk: stack.BranchRef{Branch: trunk}, - Branches: []stack.BranchRef{{Branch: "b1"}, {Branch: "b2"}, {Branch: "b3"}}, - } - if tc.merged { - s.Branches[0].PullRequest = &stack.PullRequestRef{Number: 1, Merged: true} - } - writeStackFile(t, dir, s) - before, err := os.ReadFile(filepath.Join(dir, "gh-stack")) - require.NoError(t, err) - mock := newRebaseMock(dir, "b2") - mock.BranchExistsFn = func(name string) (bool, error) { return name != "origin/main", nil } - owner := mockForeignOwner(t, mock, dir, "b2", tc.owner) - forbidRewriteMutations(t, mock) - restore := git.SetOps(mock) - defer restore() - cfg, outR, errR := config.NewTestConfig() - cfg.GitHubClientOverride = &github.MockClient{} - - require.ErrorIs(t, runRebase(cfg, &tc.opts), ErrInvalidArgs) - - _, output := commandOutput(t, cfg, outR, errR) - assert.Contains(t, output, owner) - assert.Contains(t, output, "cross-worktree rebase and sync are not supported yet") - after, err := os.ReadFile(filepath.Join(dir, "gh-stack")) - require.NoError(t, err) - assert.Equal(t, before, after) - assert.NoFileExists(t, filepath.Join(dir, rebaseStateFile)) - }) - } -} - -func TestRebase_InteractiveSelectionDoesNotCheckoutBeforeRefusal(t *testing.T) { - dir := t.TempDir() - require.NoError(t, stack.Save(dir, &stack.StackFile{ - SchemaVersion: 1, - Stacks: []stack.Stack{ - {Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "foreign"}, {Branch: "available"}}}, - {Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "independent"}}}, - }, - })) - mock := newRebaseMock(dir, "main") - mockForeignOwner(t, mock, dir, "main", "foreign") - forbidRewriteMutations(t, mock) - restore := git.SetOps(mock) - defer restore() - cfg := issue250TestConfig(t) - cfg.ForceInteractive = true - cfg.SelectFn = func(string, string, []string) (int, error) { return 0, nil } - - require.ErrorIs(t, runRebase(cfg, &rebaseOptions{noTrunk: true}), ErrInvalidArgs) -} - -func TestRebase_MigrationPrecedesOwnershipRefusal(t *testing.T) { - common := t.TempDir() - private := filepath.Join(common, "worktrees", "legacy") - require.NoError(t, os.MkdirAll(private, 0755)) - writeStackFile(t, common, stack.Stack{Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "b1"}, {Branch: "b2"}}}) - writeStackFile(t, private, stack.Stack{Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "independent"}}}) - mock := newRebaseMock(common, "b2") - mockForeignOwner(t, mock, common, "b2", "b1") - forbidRewriteMutations(t, mock) - restore := git.SetOps(mock) - defer restore() - - require.ErrorIs(t, runRebase(issue250TestConfig(t), &rebaseOptions{}), ErrInvalidArgs) - - sf, err := stack.Load(common) - require.NoError(t, err) - require.Len(t, sf.Stacks, 2) - assert.FileExists(t, filepath.Join(common, "gh-stack.pre-worktree-migration")) - assert.FileExists(t, filepath.Join(private, "gh-stack.pre-worktree-migration")) - assert.NoFileExists(t, filepath.Join(private, "gh-stack")) - assert.NoFileExists(t, filepath.Join(common, rebaseStateFile)) -} - type worktreeRebaseRepo struct { amendedParentRepo parentDir string @@ -2559,91 +2491,163 @@ func setupWorktreeRebaseRepo(t *testing.T, conflict bool) worktreeRebaseRepo { return worktreeRebaseRepo{repo, parentDir, childDir} } -func TestRebase_DistributedRefusalPreservesRefsAndWorktrees(t *testing.T) { - repo := setupWorktreeRebaseRepo(t, false) - issue250WriteFile(t, repo.parentDir, "unfinished.txt", "keep working\n") - beforeRefs := issue250Git(t, repo.dir, "show-ref") - beforeCatalog, err := os.ReadFile(filepath.Join(repo.gitDir, "gh-stack")) +func setupSharedTrunkRebaseRepo(t *testing.T, conflict bool) worktreeRebaseRepo { + t.Helper() + repo := setupWorktreeRebaseRepo(t, conflict) + issue250Git(t, repo.childDir, "checkout", "--detach") + issue250Git(t, repo.dir, "branch", "independent", "main") + sf, err := stack.Load(repo.gitDir) require.NoError(t, err) - withIssue250Repo(t, repo.childDir) + sf.AddStack(stack.Stack{Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "independent"}}}) + require.NoError(t, stack.Save(repo.gitDir, sf)) + return repo +} + +func TestRebase_SharedTrunkSelectionPreservesOriginAndRange(t *testing.T) { + for _, tc := range []struct { + name string + opts rebaseOptions + want []string + }{ + {name: "whole stack", want: []string{"b1", "b2", "b3"}}, + {name: "upstack from trunk", opts: rebaseOptions{upstack: true}, want: []string{"b1", "b2", "b3"}}, + {name: "downstack from trunk", opts: rebaseOptions{downstack: true}, want: []string{"b1"}}, + {name: "explicit upstack", opts: rebaseOptions{branch: "b2", upstack: true}, want: []string{"b2", "b3"}}, + {name: "explicit downstack", opts: rebaseOptions{branch: "b2", downstack: true}, want: []string{"b1", "b2"}}, + {name: "without trunk", opts: rebaseOptions{noTrunk: true}, want: []string{"b2", "b3"}}, + } { + t.Run(tc.name, func(t *testing.T) { + dir := t.TempDir() + writeStackFileMulti(t, dir, + stack.Stack{Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "b1"}, {Branch: "b2"}, {Branch: "b3"}}}, + stack.Stack{Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "independent"}}}, + ) + current := "main" + var rebased []string + mock := newRebaseMock(dir, current) + mock.CurrentBranchFn = func() (string, error) { return current, nil } + mock.BranchExistsFn = func(string) (bool, error) { return true, nil } + mock.CheckoutBranchFn = func(branch string) error { current = branch; return nil } + mock.RebaseFn = func(string, git.RebaseOpts) error { rebased = append(rebased, current); return nil } + mock.RebaseOntoFn = func(_, _, branch string, _ git.RebaseOpts) error { + rebased = append(rebased, branch) + return nil + } + mock.IsRerereEnabledFn = func() (bool, error) { return true, nil } + restore := git.SetOps(mock) + defer restore() + cfg := issue250TestConfig(t) + cfg.ForceInteractive = true + cfg.SelectFn = func(_, _ string, _ []string) (int, error) { return 0, nil } + tc.opts.remote = "origin" - require.ErrorIs(t, runRebase(issue250TestConfig(t), &rebaseOptions{upstack: true, noTrunk: true}), ErrInvalidArgs) + require.NoError(t, runRebase(cfg, &tc.opts)) - assert.Equal(t, beforeRefs, issue250Git(t, repo.dir, "show-ref")) - afterCatalog, err := os.ReadFile(filepath.Join(repo.gitDir, "gh-stack")) - require.NoError(t, err) - assert.Equal(t, beforeCatalog, afterCatalog) - assert.Equal(t, "main", issue250Git(t, repo.dir, "branch", "--show-current")) - assert.Equal(t, "parent", issue250Git(t, repo.parentDir, "branch", "--show-current")) - assert.Equal(t, "child", issue250Git(t, repo.childDir, "branch", "--show-current")) - assert.FileExists(t, filepath.Join(repo.parentDir, "unfinished.txt")) - assert.NoFileExists(t, filepath.Join(repo.gitDir, rebaseStateFile)) + assert.Equal(t, tc.want, rebased) + assert.Equal(t, "main", current) + assert.NoFileExists(t, filepath.Join(dir, rebaseStateFile)) + }) + } } -func TestRebase_SingleOwnerLinkedWorktreeWithForeignTrunk(t *testing.T) { +func TestRebase_SharedTrunkSelectionRecoveryPreservesOrigin(t *testing.T) { + for _, abort := range []bool{false, true} { + t.Run(fmt.Sprintf("abort=%t", abort), func(t *testing.T) { + repo := setupSharedTrunkRebaseRepo(t, true) + observerHead := issue250Git(t, repo.childDir, "rev-parse", "HEAD") + withIssue250Repo(t, repo.dir) + cfg := issue250TestConfig(t) + cfg.ForceInteractive = true + selections := 0 + cfg.SelectFn = func(_, _ string, _ []string) (int, error) { selections++; return 0, nil } + cfg.ConfirmFn = func(string, bool) (bool, error) { return false, nil } + require.ErrorIs(t, runRebase(cfg, &rebaseOptions{remote: "origin"}), ErrConflict) + state, err := loadRebaseState(repo.gitDir) + require.NoError(t, err) + assert.Equal(t, "main", state.OriginalBranch) + if !abort { + issue250WriteFile(t, repo.dir, "base.txt", "resolved\n") + issue250Git(t, repo.dir, "add", "base.txt") + } + withIssue250Repo(t, repo.parentDir) + + require.NoError(t, runRebase(cfg, &rebaseOptions{abort: abort, cont: !abort})) + + assert.Equal(t, "main", issue250Git(t, repo.dir, "branch", "--show-current")) + assert.Equal(t, "parent", issue250Git(t, repo.parentDir, "branch", "--show-current")) + assert.Equal(t, observerHead, issue250Git(t, repo.childDir, "rev-parse", "HEAD")) + assert.Equal(t, 1, selections) + assert.NoFileExists(t, filepath.Join(repo.gitDir, rebaseStateFile)) + }) + } +} + +func TestRebase_WorktreesPreserveCheckouts(t *testing.T) { repo := setupWorktreeRebaseRepo(t, false) - issue250Git(t, repo.parentDir, "checkout", "--detach") - issue250WriteFile(t, repo.dir, "unfinished.txt", "leave trunk alone\n") - trunkBefore := issue250Git(t, repo.dir, "rev-parse", "main") - withIssue250Repo(t, repo.childDir) + issue250WriteFile(t, repo.dir, "unrelated.txt", "leave main alone\n") + nested := filepath.Join(repo.childDir, "nested") + require.NoError(t, os.MkdirAll(nested, 0755)) + withIssue250Repo(t, nested) + cfg := issue250TestConfig(t) - require.NoError(t, runRebase(issue250TestConfig(t), &rebaseOptions{noTrunk: true})) + require.NoError(t, runRebase(cfg, &rebaseOptions{remote: "origin"})) - assert.Equal(t, trunkBefore, issue250Git(t, repo.dir, "rev-parse", "main")) assert.Equal(t, "main", issue250Git(t, repo.dir, "branch", "--show-current")) + assert.Equal(t, "parent", issue250Git(t, repo.parentDir, "branch", "--show-current")) assert.Equal(t, "child", issue250Git(t, repo.childDir, "branch", "--show-current")) - assert.FileExists(t, filepath.Join(repo.dir, "unfinished.txt")) require.NoError(t, issue250GitMayFail(t, repo.dir, "merge-base", "--is-ancestor", "parent", "child")) assert.Error(t, issue250GitMayFail(t, repo.dir, "merge-base", "--is-ancestor", repo.oldParent, "child")) - assert.NoFileExists(t, filepath.Join(repo.gitDir, rebaseStateFile)) + data, err := os.ReadFile(filepath.Join(repo.dir, "unrelated.txt")) + require.NoError(t, err) + assert.Equal(t, "leave main alone\n", string(data)) + sf, err := stack.Load(repo.gitDir) + require.NoError(t, err) + assert.Equal(t, issue250Git(t, repo.dir, "rev-parse", "parent"), sf.Stacks[0].Branches[1].Base) + _, err = os.Stat(filepath.Join(repo.gitDir, rebaseStateFile)) + assert.ErrorIs(t, err, os.ErrNotExist) } -func TestRebase_SharedRecoveryRequiresOrigin(t *testing.T) { - for _, action := range []string{"continue", "abort"} { - t.Run(action, func(t *testing.T) { - repo := setupWorktreeRebaseRepo(t, true) - issue250Git(t, repo.parentDir, "checkout", "--detach") - childBefore := issue250Git(t, repo.dir, "rev-parse", "child") - withIssue250Repo(t, repo.childDir) - cfg := issue250TestConfig(t) - require.ErrorIs(t, runRebase(cfg, &rebaseOptions{noTrunk: true}), ErrConflict) - state, err := loadRebaseState(repo.gitDir) - require.NoError(t, err) - require.NotNil(t, state.Worktrees) - assert.Equal(t, originOnlyRebaseMode, state.ExecutionMode) - assert.True(t, worktree.SamePath(repo.childDir, state.Worktrees.Origin.Path)) - assert.Equal(t, []string{"parent", "child"}, state.StackBranches) - before, err := os.ReadFile(filepath.Join(repo.gitDir, rebaseStateFile)) - require.NoError(t, err) - beforeRefs := issue250Git(t, repo.dir, "show-ref") +func TestRebase_WorktreesDirtyTargetFailsBeforeMutation(t *testing.T) { + repo := setupWorktreeRebaseRepo(t, false) + issue250WriteFile(t, repo.parentDir, "uncommitted.txt", "preserve me\n") + parentBefore := issue250Git(t, repo.dir, "rev-parse", "parent") + childBefore := issue250Git(t, repo.dir, "rev-parse", "child") + withIssue250Repo(t, repo.childDir) + cfg := issue250TestConfig(t) - withIssue250Repo(t, repo.dir) - opts := &rebaseOptions{cont: action == "continue", abort: action == "abort"} - require.ErrorIs(t, runRebase(cfg, opts), ErrRebaseActive) - after, err := os.ReadFile(filepath.Join(repo.gitDir, rebaseStateFile)) - require.NoError(t, err) - assert.Equal(t, before, after) - assert.Equal(t, beforeRefs, issue250Git(t, repo.dir, "show-ref")) - assert.True(t, requireGitState(t, requireWorktree(t, git.CurrentOps(), repo.childDir).IsRebaseInProgress)) - assert.False(t, requireGitState(t, git.IsRebaseInProgress)) - assert.Equal(t, "main", issue250Git(t, repo.dir, "branch", "--show-current")) + assert.ErrorIs(t, runRebase(cfg, &rebaseOptions{remote: "origin"}), ErrSilent) - if opts.cont { - issue250WriteFile(t, repo.childDir, "base.txt", "resolved\n") - issue250Git(t, repo.childDir, "add", "base.txt") - } - withIssue250Repo(t, repo.childDir) - require.NoError(t, runRebase(cfg, opts)) - assert.False(t, requireGitState(t, git.IsRebaseInProgress)) - assert.Equal(t, "child", issue250Git(t, repo.childDir, "branch", "--show-current")) - assert.NoFileExists(t, filepath.Join(repo.gitDir, rebaseStateFile)) - if opts.abort { - assert.Equal(t, childBefore, issue250Git(t, repo.dir, "rev-parse", "child")) - } else { - require.NoError(t, issue250GitMayFail(t, repo.dir, "merge-base", "--is-ancestor", "parent", "child")) - } - }) - } + assert.Equal(t, parentBefore, issue250Git(t, repo.dir, "rev-parse", "parent")) + assert.Equal(t, childBefore, issue250Git(t, repo.dir, "rev-parse", "child")) + assert.FileExists(t, filepath.Join(repo.parentDir, "uncommitted.txt")) + _, err := os.Stat(filepath.Join(repo.gitDir, rebaseStateFile)) + assert.ErrorIs(t, err, os.ErrNotExist) +} + +func TestRebase_WorktreesConflictContinueFromDifferentWorktree(t *testing.T) { + repo := setupWorktreeRebaseRepo(t, true) + withIssue250Repo(t, repo.dir) + cfg := issue250TestConfig(t) + require.ErrorIs(t, runRebase(cfg, &rebaseOptions{remote: "origin"}), ErrConflict) + + state, err := loadRebaseState(repo.gitDir) + require.NoError(t, err) + require.NotNil(t, state.Worktrees) + assert.True(t, worktree.SamePath(repo.childDir, state.Worktrees.Location("child").Path)) + assert.False(t, requireGitState(t, git.IsRebaseInProgress), "the initiating main worktree must remain usable") + assert.True(t, requireGitState(t, requireWorktree(t, git.CurrentOps(), repo.childDir).IsRebaseInProgress)) + + issue250WriteFile(t, repo.childDir, "base.txt", "resolved\n") + issue250Git(t, repo.childDir, "add", "base.txt") + withIssue250Repo(t, repo.parentDir) + require.NoError(t, runRebase(cfg, &rebaseOptions{cont: true})) + + assert.Equal(t, "main", issue250Git(t, repo.dir, "branch", "--show-current")) + assert.Equal(t, "parent", issue250Git(t, repo.parentDir, "branch", "--show-current")) + assert.Equal(t, "child", issue250Git(t, repo.childDir, "branch", "--show-current")) + require.NoError(t, issue250GitMayFail(t, repo.dir, "merge-base", "--is-ancestor", "parent", "child")) + _, err = os.Stat(filepath.Join(repo.gitDir, rebaseStateFile)) + assert.ErrorIs(t, err, os.ErrNotExist) } func TestRebase_RecoveryFindsMovedOrigin(t *testing.T) { @@ -2657,38 +2661,104 @@ func TestRebase_RecoveryFindsMovedOrigin(t *testing.T) { withIssue250Repo(t, repo.dir) moved := filepath.Join(t.TempDir(), "moved child") issue250Git(t, repo.dir, "worktree", "move", repo.childDir, moved) - require.ErrorIs(t, runRebase(cfg, &rebaseOptions{abort: true}), ErrRebaseActive) assert.True(t, requireGitState(t, requireWorktree(t, git.CurrentOps(), moved).IsRebaseInProgress)) - withIssue250Repo(t, moved) require.NoError(t, runRebase(cfg, &rebaseOptions{abort: true})) - assert.False(t, requireGitState(t, git.IsRebaseInProgress)) + assert.False(t, requireGitState(t, requireWorktree(t, git.CurrentOps(), moved).IsRebaseInProgress)) assert.Equal(t, childBefore, issue250Git(t, moved, "rev-parse", "child")) assert.Equal(t, "child", issue250Git(t, moved, "branch", "--show-current")) assert.NoFileExists(t, filepath.Join(repo.gitDir, rebaseStateFile)) } -func TestRebase_RecoveryMatchesIdentityNotOriginalCheckout(t *testing.T) { - dir := t.TempDir() - target := stack.Stack{ID: "42", Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "b1"}}} - unrelated := stack.Stack{ID: "99", Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "independent", Head: "keep"}}} - require.NoError(t, stack.Save(dir, &stack.StackFile{SchemaVersion: 1, Stacks: []stack.Stack{unrelated, target}})) - mock := newRebaseMock(dir, "independent") - restore := git.SetOps(mock) - defer restore() - ctx, err := worktree.New() +func TestRebase_WorktreesAbortRetainsRecoveryForNewEdits(t *testing.T) { + repo := setupWorktreeRebaseRepo(t, true) + parentBefore := issue250Git(t, repo.dir, "rev-parse", "parent") + childBefore := issue250Git(t, repo.dir, "rev-parse", "child") + withIssue250Repo(t, repo.dir) + cfg := issue250TestConfig(t) + require.ErrorIs(t, runRebase(cfg, &rebaseOptions{remote: "origin"}), ErrConflict) + parentRebased := issue250Git(t, repo.dir, "rev-parse", "parent") + require.NotEqual(t, parentBefore, parentRebased) + issue250WriteFile(t, repo.parentDir, "new-work.txt", "new work after conflict\n") + + withIssue250Repo(t, repo.childDir) + require.ErrorIs(t, runRebase(cfg, &rebaseOptions{abort: true}), ErrSilent) + assert.Equal(t, parentRebased, issue250Git(t, repo.dir, "rev-parse", "parent")) + assert.Equal(t, childBefore, issue250Git(t, repo.dir, "rev-parse", "child")) + assert.FileExists(t, filepath.Join(repo.parentDir, "new-work.txt")) + assert.FileExists(t, filepath.Join(repo.gitDir, rebaseStateFile)) + + require.NoError(t, os.Remove(filepath.Join(repo.parentDir, "new-work.txt"))) + require.NoError(t, runRebase(cfg, &rebaseOptions{abort: true})) + assert.Equal(t, parentBefore, issue250Git(t, repo.dir, "rev-parse", "parent")) + assert.Equal(t, childBefore, issue250Git(t, repo.dir, "rev-parse", "child")) + assert.Equal(t, "main", issue250Git(t, repo.dir, "branch", "--show-current")) + _, err := os.Stat(filepath.Join(repo.gitDir, rebaseStateFile)) + assert.ErrorIs(t, err, os.ErrNotExist) +} + +func TestRebase_WorktreesUpstackDoesNotRequireDirtyLowerWorktree(t *testing.T) { + repo := setupWorktreeRebaseRepo(t, false) + parentBefore := issue250Git(t, repo.dir, "rev-parse", "parent") + issue250WriteFile(t, repo.parentDir, "unfinished.txt", "keep working\n") + withIssue250Repo(t, repo.dir) + cfg := issue250TestConfig(t) + + require.NoError(t, runRebase(cfg, &rebaseOptions{branch: "child", upstack: true, noTrunk: true})) + + assert.Equal(t, parentBefore, issue250Git(t, repo.dir, "rev-parse", "parent")) + assert.FileExists(t, filepath.Join(repo.parentDir, "unfinished.txt")) + assert.Equal(t, "main", issue250Git(t, repo.dir, "branch", "--show-current")) + require.NoError(t, issue250GitMayFail(t, repo.dir, "merge-base", "--is-ancestor", "parent", "child")) +} + +func TestRebase_WorktreesExplicitTargetRecoverySelectsCorrectStack(t *testing.T) { + repo := setupWorktreeRebaseRepo(t, true) + issue250Git(t, repo.dir, "checkout", "-b", "independent", "main") + sf, err := stack.Load(repo.gitDir) require.NoError(t, err) - state := newWorktreeRebaseState(&target, ctx, "independent", map[string]string{"b1": "old-b1"}, trunkTarget{Ref: "main", SHA: "sha-main"}, 0, 1) - state.Phase, state.ConflictBranch = "conflict", "b1" - require.NoError(t, saveRebaseState(dir, state)) + sf.AddStack(stack.Stack{Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "independent"}}}) + require.NoError(t, stack.Save(repo.gitDir, sf)) + withIssue250Repo(t, repo.dir) + cfg := issue250TestConfig(t) - require.NoError(t, runRebase(issue250TestConfig(t), &rebaseOptions{cont: true})) + require.ErrorIs(t, runRebase(cfg, &rebaseOptions{branch: "child", upstack: true, remote: "origin"}), ErrConflict) + issue250WriteFile(t, repo.childDir, "base.txt", "resolved\n") + issue250Git(t, repo.childDir, "add", "base.txt") + require.NoError(t, runRebase(cfg, &rebaseOptions{cont: true})) - sf, err := stack.Load(dir) + assert.Equal(t, "independent", issue250Git(t, repo.dir, "branch", "--show-current")) + sf, err = stack.Load(repo.gitDir) require.NoError(t, err) - assert.Equal(t, unrelated, sf.Stacks[0]) - assert.Equal(t, "sha-b1", sf.Stacks[1].Branches[0].Head) - assert.NoFileExists(t, filepath.Join(dir, rebaseStateFile)) + require.Len(t, sf.Stacks, 2) + assert.Equal(t, []string{"parent", "child"}, sf.Stacks[0].BranchNames()) + assert.Equal(t, []string{"independent"}, sf.Stacks[1].BranchNames()) + require.NoError(t, issue250GitMayFail(t, repo.dir, "merge-base", "--is-ancestor", "parent", "child")) +} + +func TestRebase_ContinueWithRemainingBranchInConflictWorktree(t *testing.T) { + repo := setupWorktreeRebaseRepo(t, true) + issue250Git(t, repo.dir, "worktree", "remove", repo.parentDir) + issue250Git(t, repo.dir, "worktree", "remove", repo.childDir) + issue250Git(t, repo.dir, "branch", "grandchild", "child") + sf, err := stack.Load(repo.gitDir) + require.NoError(t, err) + child := issue250Git(t, repo.dir, "rev-parse", "child") + sf.Stacks[0].Branches = append(sf.Stacks[0].Branches, stack.BranchRef{Branch: "grandchild", Head: child, Base: child}) + require.NoError(t, stack.Save(repo.gitDir, sf)) + withIssue250Repo(t, repo.dir) + cfg := issue250TestConfig(t) + + require.ErrorIs(t, runRebase(cfg, &rebaseOptions{remote: "origin"}), ErrConflict) + issue250WriteFile(t, repo.dir, "base.txt", "resolved\n") + issue250Git(t, repo.dir, "add", "base.txt") + require.NoError(t, runRebase(cfg, &rebaseOptions{cont: true})) + + require.NoError(t, issue250GitMayFail(t, repo.dir, "merge-base", "--is-ancestor", "child", "grandchild")) + assert.Equal(t, "main", issue250Git(t, repo.dir, "branch", "--show-current")) + assert.False(t, requireGitState(t, git.IsRebaseInProgress)) + _, err = os.Stat(filepath.Join(repo.gitDir, rebaseStateFile)) + assert.ErrorIs(t, err, os.ErrNotExist) } func TestRebase_LegacyPrivateRecoveryKeepsOriginalCatalog(t *testing.T) { @@ -2732,18 +2802,21 @@ func TestRebase_AbortRetainsPartialRestore(t *testing.T) { mock.CurrentBranchFn = func() (string, error) { return current, nil } mock.RevParseFn = func(ref string) (string, error) { return refs[ref], nil } mock.CheckoutBranchFn = func(branch string) error { current = branch; return nil } - mock.ResetHardFn = func(sha string) error { - if current == "b2" && fail { - return errors.New("reset failed") + mock.ResetHardFn = func(sha string) error { refs[current] = sha; return nil } + mock.UpdateBranchRefFn = func(branch, sha string) error { + if branch == "b2" && fail { + return errors.New("ref restore failed") } - refs[current] = sha + refs[branch] = sha return nil } restore := git.SetOps(mock) defer restore() ctx, err := worktree.New() require.NoError(t, err) + ctx.Touched = map[string]string{"b1": "rebased-b1", "b2": "rebased-b2"} state := newWorktreeRebaseState(&s, ctx, "b1", map[string]string{"b1": "old-b1", "b2": "old-b2"}, trunkTarget{}, 0, 2) + state.CurrentBranchIndex = 2 require.NoError(t, saveRebaseState(dir, state)) cfg := issue250TestConfig(t) @@ -2751,6 +2824,7 @@ func TestRebase_AbortRetainsPartialRestore(t *testing.T) { retained, err := loadRebaseState(dir) require.NoError(t, err) assert.Equal(t, "restoring", retained.Phase) + assert.Equal(t, map[string]string{"b2": "rebased-b2"}, retained.Worktrees.Touched) assert.Equal(t, "old-b1", refs["b1"]) assert.Equal(t, "rebased-b2", refs["b2"]) @@ -2767,12 +2841,14 @@ func TestRebase_CompletedJournalSurvivesCatalogSaveFailure(t *testing.T) { writeStackFile(t, dir, s) mock := newRebaseMock(dir, "b1") mock.RebaseContinueFn = func(git.RebaseOpts) error { t.Fatal("must not repeat a completed native rebase"); return nil } + mock.RebaseFn = func(string, git.RebaseOpts) error { t.Fatal("must not repeat a completed rebase"); return nil } restore := git.SetOps(mock) defer restore() ctx, err := worktree.New() require.NoError(t, err) + ctx.Touched["b1"] = "sha-b1" state := newWorktreeRebaseState(&s, ctx, "b1", map[string]string{"b1": "old-b1"}, trunkTarget{Ref: "main", SHA: "sha-main"}, 0, 1) - state.Phase, state.ConflictBranch = "conflict", "b1" + state.CurrentBranchIndex = state.EndIndex require.NoError(t, saveRebaseState(dir, state)) lock, err := stack.Lock(dir) require.NoError(t, err) @@ -2786,12 +2862,180 @@ func TestRebase_CompletedJournalSurvivesCatalogSaveFailure(t *testing.T) { retained, err := loadRebaseState(dir) require.NoError(t, err) assert.Equal(t, "complete", retained.Phase) + assert.Equal(t, map[string]string{"b1": "sha-b1"}, retained.Worktrees.Touched) lock.Unlock() require.NoError(t, runRebase(cfg, &rebaseOptions{cont: true})) assert.NoFileExists(t, filepath.Join(dir, rebaseStateFile)) } +func TestRebase_CompletedRecoveryPreservesNewWork(t *testing.T) { + for _, action := range []string{"continue", "abort"} { + for _, busy := range []bool{false, true} { + t.Run(fmt.Sprintf("%s/busy=%t", action, busy), func(t *testing.T) { + dir := t.TempDir() + s := stack.Stack{Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "b1"}}} + writeStackFile(t, dir, s) + mock := newRebaseMock(dir, "b1") + mock.IsRebaseInProgressFn = func() (bool, error) { return busy, nil } + mock.HasUncommittedChangesFn = func() (bool, error) { return !busy, nil } + mock.RebaseAbortFn = func() error { t.Fatal("must not abort a new Git operation"); return nil } + mock.RebaseContinueFn = func(git.RebaseOpts) error { t.Fatal("must not continue a new Git operation"); return nil } + forbidRewriteMutations(t, mock) + restore := git.SetOps(mock) + defer restore() + ctx, err := worktree.New() + require.NoError(t, err) + ctx.Touched["b1"] = "sha-b1" + state := newWorktreeRebaseState(&s, ctx, "b1", map[string]string{"b1": "old-b1"}, trunkTarget{}, 0, 1) + state.Phase, state.CurrentBranchIndex = "complete", state.EndIndex + require.NoError(t, saveRebaseState(dir, state)) + + err = runRebase(issue250TestConfig(t), &rebaseOptions{cont: action == "continue", abort: action == "abort"}) + if action == "abort" { + require.ErrorIs(t, err, ErrSilent) + retained, err := loadRebaseState(dir) + require.NoError(t, err) + assert.Equal(t, "restoring", retained.Phase) + assert.Equal(t, ctx.Touched, retained.Worktrees.Touched) + } else { + require.NoError(t, err) + assert.NoFileExists(t, filepath.Join(dir, rebaseStateFile)) + } + assert.Equal(t, busy, requireGitState(t, mock.IsRebaseInProgress)) + dirty, err := mock.HasUncommittedChanges() + require.NoError(t, err) + assert.Equal(t, !busy, dirty) + }) + } + } +} + +func forbidRewriteMutations(t *testing.T, mock *git.MockOps) { + t.Helper() + mock.CheckoutBranchFn = func(string) error { t.Fatal("unexpected checkout"); return nil } + mock.CreateBranchFn = func(string, string) error { t.Fatal("unexpected branch creation"); return nil } + mock.UpdateBranchRefFn = func(string, string) error { t.Fatal("unexpected ref update"); return nil } + mock.ResetHardFn = func(string) error { t.Fatal("unexpected reset"); return nil } + mock.MergeFFFn = func(string) error { t.Fatal("unexpected fast-forward"); return nil } + mock.RebaseFn = func(string, git.RebaseOpts) error { t.Fatal("unexpected rebase"); return nil } + mock.RebaseOntoFn = func(string, string, string, git.RebaseOpts) error { + t.Fatal("unexpected rebase onto") + return nil + } + mock.PushFn = func(string, []string, bool, bool) error { t.Fatal("unexpected push"); return nil } + mock.DeleteBranchFn = func(string, bool) error { t.Fatal("unexpected branch deletion"); return nil } + mock.DeleteTrackingRefFn = func(string, string) error { t.Fatal("unexpected tracking ref deletion"); return nil } +} + +func TestRebase_RecoveryRejectsDifferentExecutionLifecycle(t *testing.T) { + actions := []struct { + name string + run func(*config.Config, string) error + want error + }{ + {"continue", func(cfg *config.Config, _ string) error { return runRebase(cfg, &rebaseOptions{cont: true}) }, ErrRebaseActive}, + {"abort", func(cfg *config.Config, _ string) error { return runRebase(cfg, &rebaseOptions{abort: true}) }, ErrRebaseActive}, + {"sync", func(cfg *config.Config, _ string) error { return runSync(cfg, &syncOptions{}) }, ErrRebaseActive}, + {"direct continue", continueRebase, ErrSilent}, + {"direct abort", abortRebase, ErrSilent}, + } + for _, mode := range []string{originOnlyRebaseMode, "future-lifecycle"} { + for _, withContext := range []bool{false, true} { + for _, phase := range []string{"applying", "conflict", "complete", "restoring"} { + for _, action := range actions { + t.Run(fmt.Sprintf("%s/context=%t/%s/%s", mode, withContext, phase, action.name), func(t *testing.T) { + dir := t.TempDir() + private := filepath.Join(dir, "worktrees", "caller") + require.NoError(t, os.MkdirAll(private, 0755)) + writeStackFile(t, dir, stack.Stack{Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "b1"}}}) + writeStackFile(t, private, stack.Stack{Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "independent"}}}) + mock := newRebaseMock(private, "b1") + mock.CommonDirFn = func() (string, error) { return dir, nil } + forbidRewriteMutations(t, mock) + mock.CurrentBranchFn = func() (string, error) { t.Fatal("must reject before reading native checkout state"); return "", nil } + mock.IsRebaseInProgressFn = func() (bool, error) { t.Fatal("must reject before reading native rebase state"); return false, nil } + mock.ForWorktreeFn = func(string) (git.Ops, error) { + t.Fatal("must reject before resolving a recorded worktree") + return nil, nil + } + mock.RebaseAbortFn = func() error { t.Fatal("must not abort an incompatible lifecycle"); return nil } + mock.RebaseContinueFn = func(git.RebaseOpts) error { t.Fatal("must not continue an incompatible lifecycle"); return nil } + restore := git.SetOps(mock) + defer restore() + state := &rebaseState{ + ExecutionMode: mode, Phase: phase, OriginalBranch: "b1", ConflictBranch: "b1", + OriginalRefs: map[string]string{"b1": "before"}, EndIndex: 1, + } + origin := filepath.Join(dir, "recorded-origin") + if withContext { + state.Worktrees = &worktree.Context{Origin: worktree.Location{Path: origin, ID: "."}} + } + require.NoError(t, saveRebaseState(dir, state)) + paths := []string{filepath.Join(dir, rebaseStateFile), filepath.Join(dir, "gh-stack"), filepath.Join(private, "gh-stack")} + before := make([][]byte, len(paths)) + for i, path := range paths { + var err error + before[i], err = os.ReadFile(path) + require.NoError(t, err) + } + _, err := loadRebaseState(dir) + require.Error(t, err) + cfg, outR, errR := config.NewTestConfig() + + require.ErrorIs(t, action.run(cfg, dir), action.want) + + _, output := commandOutput(t, cfg, outR, errR) + assert.NotContains(t, output, "no rebase in progress") + if mode == originOnlyRebaseMode { + assert.Contains(t, output, "matching layer3 gh-stack build") + } else { + assert.Contains(t, output, `unsupported rebase execution mode "future-lifecycle"`) + } + assert.Contains(t, output, "continue or abort") + assert.Contains(t, output, "recovery state was retained") + if withContext { + assert.Contains(t, output, fmt.Sprintf("%q", origin)) + } + for i, path := range paths { + after, err := os.ReadFile(path) + require.NoError(t, err) + assert.Equal(t, before[i], after, path) + } + assert.Nil(t, cfg.StackMutation) + assert.NoFileExists(t, filepath.Join(dir, "gh-stack-migration")) + assert.NoFileExists(t, filepath.Join(dir, "gh-stack.pre-worktree-migration")) + assert.NoFileExists(t, filepath.Join(private, "gh-stack.pre-worktree-migration")) + }) + } + } + } + } +} + +func TestLoadRebaseState_UnmarkedLifecycles(t *testing.T) { + for _, withContext := range []bool{false, true} { + t.Run(fmt.Sprintf("context=%t", withContext), func(t *testing.T) { + dir := t.TempDir() + s := stack.Stack{Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "b1"}}} + var ctx *worktree.Context + if withContext { + ctx = &worktree.Context{Origin: worktree.Location{Path: dir, ID: "."}} + } + state := newWorktreeRebaseState(&s, ctx, "b1", map[string]string{"b1": "before"}, trunkTarget{}, 0, 1) + require.NoError(t, saveRebaseState(dir, state)) + + loaded, err := loadRebaseState(dir) + + require.NoError(t, err) + assert.Equal(t, state, loaded) + data, err := os.ReadFile(filepath.Join(dir, rebaseStateFile)) + require.NoError(t, err) + assert.NotContains(t, string(data), "executionMode") + }) + } +} + func TestRebase_LegacyContinuePersistsCatalogUnderOperationLock(t *testing.T) { dir := t.TempDir() writeStackFile(t, dir, stack.Stack{ @@ -2824,77 +3068,281 @@ func TestRebase_LegacyContinuePersistsCatalogUnderOperationLock(t *testing.T) { assert.ErrorIs(t, err, os.ErrNotExist) } -func TestRebase_CompletedRecoveryPreservesNewWork(t *testing.T) { - for _, action := range []string{"continue", "abort"} { - for _, busy := range []bool{false, true} { - t.Run(fmt.Sprintf("%s/busy=%t", action, busy), func(t *testing.T) { - dir := t.TempDir() - s := stack.Stack{Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "b1"}}} - writeStackFile(t, dir, s) - mock := newRebaseMock(dir, "b1") - mock.IsRebaseInProgressFn = func() (bool, error) { return busy, nil } - mock.HasUncommittedChangesFn = func() (bool, error) { return !busy, nil } - mock.RebaseAbortFn = func() error { t.Fatal("must not abort a new Git operation"); return nil } - forbidRewriteMutations(t, mock) - restore := git.SetOps(mock) - defer restore() - ctx, err := worktree.New() - require.NoError(t, err) - state := newWorktreeRebaseState(&s, ctx, "b1", map[string]string{"b1": "old-b1"}, trunkTarget{}, 0, 1) - state.Phase = "complete" - require.NoError(t, saveRebaseState(dir, state)) - before, err := os.ReadFile(filepath.Join(dir, rebaseStateFile)) - require.NoError(t, err) +func TestRebase_LegacyPhaseGuards(t *testing.T) { + for _, tc := range []struct { + phase string + abort bool + }{ + {phase: "applying"}, + {phase: "restoring"}, + {phase: "unknown"}, + {phase: "unknown", abort: true}, + } { + t.Run(fmt.Sprintf("%s/abort=%t", tc.phase, tc.abort), func(t *testing.T) { + dir := t.TempDir() + writeStackFile(t, dir, stack.Stack{ + Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "b1"}}, + }) + state := &rebaseState{ + Phase: tc.phase, OriginalBranch: "b1", ConflictBranch: "b1", + OriginalRefs: map[string]string{"b1": "before"}, + } + require.NoError(t, saveRebaseState(dir, state)) + before, err := os.ReadFile(filepath.Join(dir, rebaseStateFile)) + require.NoError(t, err) + mock := newRebaseMock(dir, "b1") + forbidRewriteMutations(t, mock) + mock.IsRebaseInProgressFn = func() (bool, error) { + t.Error("phase rejection must precede native state queries") + return false, nil + } + mock.RebaseContinueFn = func(git.RebaseOpts) error { t.Error("unexpected native continuation"); return nil } + mock.RebaseAbortFn = func() error { t.Error("unexpected native abort"); return nil } + restore := git.SetOps(mock) + defer restore() - require.ErrorIs(t, runRebase(issue250TestConfig(t), &rebaseOptions{cont: action == "continue", abort: action == "abort"}), ErrSilent) + err = runRebase(issue250TestConfig(t), &rebaseOptions{cont: !tc.abort, abort: tc.abort}) - after, err := os.ReadFile(filepath.Join(dir, rebaseStateFile)) - require.NoError(t, err) - assert.Equal(t, before, after) - }) - } + require.Error(t, err) + assert.Contains(t, err.Error(), tc.phase) + if tc.phase != "unknown" { + assert.Contains(t, err.Error(), "gh stack rebase --abort") + } + after, err := os.ReadFile(filepath.Join(dir, rebaseStateFile)) + require.NoError(t, err) + assert.Equal(t, before, after) + }) } } -func TestRebase_RecoveryRejectsDifferentExecutionLifecycle(t *testing.T) { - for _, mode := range []string{"", "future-lifecycle", originOnlyRebaseMode} { - for _, action := range []string{"continue", "abort"} { - t.Run(mode+"/"+action, func(t *testing.T) { - dir := t.TempDir() - mock := newRebaseMock(dir, "b1") - forbidRewriteMutations(t, mock) - mock.RebaseAbortFn = func() error { t.Fatal("must not abort an incompatible lifecycle"); return nil } - mock.RebaseContinueFn = func(git.RebaseOpts) error { - t.Fatal("must not continue an incompatible lifecycle") - return nil +func TestRebase_LegacyCompleteOnlyPublishes(t *testing.T) { + dir := t.TempDir() + writeStackFile(t, dir, stack.Stack{ + Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "b1"}, {Branch: "b2"}}, + }) + state := &rebaseState{ + Phase: "complete", OriginalBranch: "b1", ConflictBranch: "b1", + RemainingBranches: []string{"b2"}, OriginalRefs: map[string]string{"b1": "old-b1", "b2": "old-b2"}, + } + require.NoError(t, saveRebaseState(dir, state)) + mock := newRebaseMock(dir, "b1") + forbidRewriteMutations(t, mock) + mock.IsRebaseInProgressFn = func() (bool, error) { return true, nil } + mock.RebaseContinueFn = func(git.RebaseOpts) error { + t.Error("must not continue a new unrelated native operation") + return assert.AnError + } + restore := git.SetOps(mock) + defer restore() + + require.NoError(t, runRebase(issue250TestConfig(t), &rebaseOptions{cont: true})) + + assert.True(t, requireGitState(t, mock.IsRebaseInProgress)) + assert.NoFileExists(t, filepath.Join(dir, rebaseStateFile)) +} + +func TestRebase_LegacyCompleteAbortPreservesNewNativeOperation(t *testing.T) { + dir := t.TempDir() + state := &rebaseState{ + Phase: "complete", OriginalBranch: "b1", OriginalRefs: map[string]string{"b1": "before"}, + } + require.NoError(t, saveRebaseState(dir, state)) + before, err := os.ReadFile(filepath.Join(dir, rebaseStateFile)) + require.NoError(t, err) + mock := newRebaseMock(dir, "b1") + forbidRewriteMutations(t, mock) + mock.IsRebaseInProgressFn = func() (bool, error) { return true, nil } + mock.RebaseAbortFn = func() error { + t.Fatal("must not abort a native operation started after completion") + return nil + } + restore := git.SetOps(mock) + defer restore() + + require.Error(t, runRebase(issue250TestConfig(t), &rebaseOptions{abort: true})) + + after, err := os.ReadFile(filepath.Join(dir, rebaseStateFile)) + require.NoError(t, err) + assert.Equal(t, before, after) +} + +func TestRebase_LegacyAbortRetainsFailures(t *testing.T) { + for _, failure := range []string{"native abort", "branch checkout", "reset", "original checkout", "branch lookup"} { + t.Run(failure, func(t *testing.T) { + dir := t.TempDir() + refs := map[string]string{"b1": "new-b1", "b2": "new-b2"} + state := &rebaseState{ + Phase: "conflict", OriginalBranch: "main", ConflictBranch: "b2", + OriginalRefs: map[string]string{"b1": "old-b1", "b2": "old-b2"}, + } + require.NoError(t, saveRebaseState(dir, state)) + current, failing := "b2", true + inProgress := failure == "native abort" + mock := newRebaseMock(dir, current) + mock.CurrentBranchFn = func() (string, error) { return current, nil } + mock.BranchExistsFn = func(branch string) (bool, error) { + if failing && failure == "branch lookup" && branch == "b2" { + return false, assert.AnError } - restore := git.SetOps(mock) - defer restore() - state := &rebaseState{ - ExecutionMode: mode, Phase: "conflict", OriginalBranch: "b1", ConflictBranch: "b1", - OriginalRefs: map[string]string{"b1": "before"}, + return true, nil + } + mock.RevParseFn = func(ref string) (string, error) { return refs[ref], nil } + mock.IsRebaseInProgressFn = func() (bool, error) { return inProgress, nil } + mock.RebaseAbortFn = func() error { + if failing && failure == "native abort" { + return assert.AnError } - if mode != originOnlyRebaseMode { - state.Worktrees = &worktree.Context{Origin: worktree.Location{Path: dir, ID: "."}} + inProgress = false + return nil + } + mock.CheckoutBranchFn = func(branch string) error { + if failing && ((failure == "branch checkout" && branch == "b2") || (failure == "original checkout" && branch == "main")) { + return assert.AnError } - require.NoError(t, saveRebaseState(dir, state)) - before, err := os.ReadFile(filepath.Join(dir, rebaseStateFile)) - require.NoError(t, err) - cfg, outR, errR := config.NewTestConfig() + current = branch + return nil + } + mock.ResetHardFn = func(sha string) error { + if failing && failure == "reset" && current == "b2" { + return assert.AnError + } + refs[current] = sha + return nil + } + restore := git.SetOps(mock) + defer restore() + cfg := issue250TestConfig(t) - require.ErrorIs(t, runRebase(cfg, &rebaseOptions{cont: action == "continue", abort: action == "abort"}), ErrSilent) + require.Error(t, runRebase(cfg, &rebaseOptions{abort: true})) + retained, err := loadRebaseState(dir) + require.NoError(t, err, "failure must retain recovery state") + assert.Equal(t, state.OriginalRefs, retained.OriginalRefs) + if failure == "branch lookup" { + assert.Equal(t, state, retained, "query errors must precede journal changes") + } else { + assert.Equal(t, "restoring", retained.Phase) + } - _, output := commandOutput(t, cfg, outR, errR) - if mode == originOnlyRebaseMode { - assert.Contains(t, output, "no recorded worktree") - } else { - assert.Contains(t, output, "matching gh-stack build") - assert.Contains(t, output, fmt.Sprintf("%q", dir)) + failing = false + require.NoError(t, runRebase(cfg, &rebaseOptions{abort: true})) + assert.Equal(t, state.OriginalRefs, refs) + assert.Equal(t, "main", current) + assert.NoFileExists(t, filepath.Join(dir, rebaseStateFile)) + }) + } +} + +func TestRebase_LegacyContinueRollbackRetainsJournal(t *testing.T) { + for _, reason := range []string{"cascade error", "verification failure"} { + t.Run(reason, func(t *testing.T) { + dir := t.TempDir() + writeStackFile(t, dir, stack.Stack{ + Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "b1"}, {Branch: "b2"}}, + }) + state := &rebaseState{ + OriginalBranch: "b1", ConflictBranch: "b1", RemainingBranches: []string{"b2"}, + OriginalRefs: map[string]string{"b1": "old-b1", "b2": "old-b2"}, + } + require.NoError(t, saveRebaseState(dir, state)) + refs := map[string]string{"b1": "new-b1", "b2": "new-b2"} + current, failing := "b1", true + mock := newRebaseMock(dir, current) + mock.CurrentBranchFn = func() (string, error) { return current, nil } + mock.BranchExistsFn = func(string) (bool, error) { return true, nil } + mock.RevParseFn = func(ref string) (string, error) { return refs[ref], nil } + mock.CheckoutBranchFn = func(branch string) error { current = branch; return nil } + mock.ResetHardFn = func(sha string) error { + if failing && current == "b2" { + return assert.AnError } - after, err := os.ReadFile(filepath.Join(dir, rebaseStateFile)) - require.NoError(t, err) - assert.Equal(t, before, after) + refs[current] = sha + return nil + } + mock.RebaseOntoFn = func(_, _, branch string, _ git.RebaseOpts) error { + current = branch + if reason == "cascade error" { + return &git.RebaseStartError{Err: assert.AnError} + } + return nil + } + mock.IsAncestorFn = func(base, branch string) (bool, error) { + return !(reason == "verification failure" && base == "main" && branch == "b1"), nil + } + restore := git.SetOps(mock) + defer restore() + cfg := issue250TestConfig(t) + + require.Error(t, runRebase(cfg, &rebaseOptions{cont: true})) + retained, err := loadRebaseState(dir) + require.NoError(t, err) + assert.Equal(t, "restoring", retained.Phase) + assert.Equal(t, "old-b1", refs["b1"]) + assert.Equal(t, "new-b2", refs["b2"]) + + failing = false + require.NoError(t, runRebase(cfg, &rebaseOptions{abort: true})) + assert.Equal(t, state.OriginalRefs, refs) + assert.NoFileExists(t, filepath.Join(dir, rebaseStateFile)) + }) + } +} + +func TestRebase_LegacyCompletionRetainsRetryState(t *testing.T) { + for _, failure := range []string{"catalog save", "original checkout"} { + t.Run(failure, func(t *testing.T) { + dir := t.TempDir() + writeStackFile(t, dir, stack.Stack{ + Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "b1"}}, }) - } + require.NoError(t, saveRebaseState(dir, &rebaseState{ + OriginalBranch: "main", ConflictBranch: "b1", OriginalRefs: map[string]string{"b1": "before"}, + })) + current, failing, inProgress := "b1", true, true + continues := 0 + mock := newRebaseMock(dir, current) + mock.CurrentBranchFn = func() (string, error) { return current, nil } + mock.IsRebaseInProgressFn = func() (bool, error) { return inProgress, nil } + mock.RebaseContinueFn = func(git.RebaseOpts) error { + continues++ + inProgress = false + return nil + } + mock.CheckoutBranchFn = func(branch string) error { + if failing && failure == "original checkout" { + return assert.AnError + } + current = branch + return nil + } + restore := git.SetOps(mock) + defer restore() + oldTimeout := stack.LockTimeout + stack.LockTimeout = 0 + defer func() { stack.LockTimeout = oldTimeout }() + var lock *stack.FileLock + if failure == "catalog save" { + var err error + lock, err = stack.Lock(dir) + require.NoError(t, err) + defer lock.Unlock() + } + cfg := issue250TestConfig(t) + + require.Error(t, runRebase(cfg, &rebaseOptions{cont: true})) + retained, err := loadRebaseState(dir) + require.NoError(t, err) + assert.Equal(t, "complete", retained.Phase) + assert.Equal(t, 1, continues) + if lock != nil { + lock.Unlock() + inProgress = true + } + failing = false + + require.NoError(t, runRebase(cfg, &rebaseOptions{cont: true})) + assert.Equal(t, 1, continues, "completed publication must not repeat native continuation") + assert.Equal(t, "main", current) + assert.NoFileExists(t, filepath.Join(dir, rebaseStateFile)) + }) } } diff --git a/cmd/rebase_worktree.go b/cmd/rebase_worktree.go new file mode 100644 index 0000000..cbb36c2 --- /dev/null +++ b/cmd/rebase_worktree.go @@ -0,0 +1,346 @@ +package cmd + +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/worktree" +) + +func rebaseBranchNames(branches []stack.BranchRef) []string { + var names []string + for _, branch := range branches { + if !branch.IsSkipped() { + names = append(names, branch.Branch) + } + } + return names +} + +func newWorktreeRebaseState(s *stack.Stack, ctx *worktree.Context, originalBranch string, refs map[string]string, trunk trunkTarget, start, end int) *rebaseState { + snapshot := *s + snapshot.Branches = append([]stack.BranchRef{}, s.Branches...) + return &rebaseState{ + Phase: "applying", + Worktrees: ctx, + StackID: s.ID, + StackTrunk: s.Trunk.Branch, + StackBranches: s.BranchNames(), + OriginalStack: &snapshot, + OriginalBranch: originalBranch, + OriginalRefs: refs, + CurrentBranchIndex: start, + StartIndex: start, + EndIndex: end, + TrunkRef: trunk.Ref, + TrunkSHA: trunk.SHA, + } +} + +func printWorktreeConflict(cfg *config.Config, state *rebaseState, base string) { + branch := state.Worktrees.Pending + if branch == "" { + branch = state.ConflictBranch + } + ops, err := state.Worktrees.Ops(branch) + if err != nil { + cfg.Errorf("%s", err) + return + } + printConflictDetailsAt(cfg, ops, state.Worktrees.Location(branch).Path, base, "gh stack rebase --continue") +} + +func rollbackWorktreeRebase(cfg *config.Config, dir string, state *rebaseState) error { + ctx := state.Worktrees + if _, err := ctx.OriginOps(); err != nil { + cfg.Errorf("%s", err) + return err + } + branches := make([]string, 0, len(ctx.Touched)+1) + for branch := range ctx.Touched { + branches = append(branches, branch) + } + if ctx.Pending != "" { + branches = append(branches, ctx.Pending) + } + var pendingOps git.Ops + var inProgress bool + for _, branch := range branches { + ops, err := ctx.Ops(branch) + if err != nil { + cfg.Errorf("%s", err) + return err + } + if _, err := ops.BranchExists(branch); err != nil { + cfg.Errorf("checking branch %s before restoring: %s", branch, err) + return fmt.Errorf("checking branch %s before restoring: %w", branch, err) + } + if branch == ctx.Pending { + pendingOps = ops + inProgress, err = ops.IsRebaseInProgress() + if err != nil { + cfg.Errorf("checking rebase state: %s", err) + return fmt.Errorf("checking rebase state: %w", err) + } + } + } + state.Phase = "restoring" + var err error + if inProgress { + err = pendingOps.RebaseAbort() + } + if err == nil { + err = errors.Join(ctx.Restore(state.OriginalRefs), ctx.RestoreOrigin(state.OriginalBranch)) + } + if err == nil && state.OriginalStack != nil { + err = restoreWorktreeRebaseMetadata(dir, state) + } + if err != nil { + if saveErr := saveRebaseState(dir, state); saveErr != nil { + err = errors.Join(err, saveErr) + } + cfg.Errorf("Could not fully restore the stack; recovery state was retained: %v", err) + return err + } + if err := clearRebaseState(dir); err != nil { + cfg.Errorf("Branches restored, but could not clear recovery state: %v", err) + return err + } + return nil +} + +func restoreWorktreeRebaseMetadata(dir string, state *rebaseState) error { + sf, err := stack.Load(dir) + if err != nil { + return err + } + target, err := rebaseStackFromState(sf, state) + if err != nil { + return err + } + if !slices.Equal(state.OriginalStack.BranchNames(), target.BranchNames()) { + return fmt.Errorf("original stack snapshot does not match the recovery target") + } + target.Trunk.Head = state.OriginalStack.Trunk.Head + for i, before := range state.OriginalStack.Branches { + target.Branches[i].Base = before.Base + target.Branches[i].Head = before.Head + if sha, err := git.RevParse(before.Branch); err == nil { + target.Branches[i].Head = sha + } else if !before.IsMerged() { + return fmt.Errorf("reading restored branch %s: %w", before.Branch, err) + } + } + return stack.Save(dir, sf) +} + +func rebaseStackFromState(sf *stack.StackFile, state *rebaseState) (*stack.Stack, error) { + switch state.Phase { + case "applying", "conflict", "complete", "restoring": + default: + return nil, fmt.Errorf("unknown saved rebase phase %q", state.Phase) + } + var target *stack.Stack + for i := range sf.Stacks { + s := &sf.Stacks[i] + if s.Trunk.Branch != state.StackTrunk || !slices.Equal(s.BranchNames(), state.StackBranches) { + continue + } + if (state.Phase == "applying" || state.Phase == "conflict") && state.StackID != "" && s.ID != "" && state.StackID != s.ID { + continue + } + if target != nil { + return nil, fmt.Errorf("saved rebase matches multiple stacks; resolve the catalog before continuing") + } + target = s + } + if target == nil { + return nil, fmt.Errorf("the stack changed since this rebase started; restore its original membership or run gh stack rebase --abort") + } + if state.StartIndex < 0 || state.EndIndex > len(target.Branches) || + state.StartIndex > state.EndIndex || state.CurrentBranchIndex < state.StartIndex || + state.CurrentBranchIndex > state.EndIndex { + return nil, fmt.Errorf("invalid branch range in saved rebase state") + } + return target, nil +} + +func continueWorktreeRebase(cfg *config.Config, dir string, state *rebaseState) error { + if state.Phase == "restoring" { + return fmt.Errorf("restoration is incomplete; run gh stack rebase --abort to finish restoring the stack") + } + sf, err := stack.Load(dir) + if err != nil { + return err + } + s, err := rebaseStackFromState(sf, state) + if err != nil { + return err + } + ctx := state.Worktrees + _ = syncStackPRs(cfg, s) + if state.Phase != "complete" { + remainingStart := state.CurrentBranchIndex + if ctx.Pending != "" { + remainingStart++ + } + if remainingStart > state.EndIndex { + return fmt.Errorf("invalid pending branch in saved rebase state") + } + required := rebaseBranchNames(s.Branches[remainingStart:state.EndIndex]) + if ctx.Pending != "" { + if _, err := ctx.Ops(ctx.Pending); err != nil { + return err + } + pendingPath := ctx.Location(ctx.Pending).Path + otherWorktrees := required[:0] + for _, branch := range required { + if _, err := ctx.Ops(branch); err != nil { + return err + } + if !worktree.SamePath(ctx.Location(branch).Path, pendingPath) { + otherWorktrees = append(otherWorktrees, branch) + } + } + required = otherWorktrees + } + // The pending worktree is intentionally busy. Its remaining branches + // are checked by Prepare after native continuation has completed. + if err := ctx.Preflight(required); err != nil { + return err + } + if ctx.Pending != "" { + branch := ctx.Pending + if state.CurrentBranchIndex >= len(s.Branches) || s.Branches[state.CurrentBranchIndex].Branch != branch { + return fmt.Errorf("saved conflict branch does not match the stack") + } + ops, err := ctx.Ops(branch) + if err != nil { + return err + } + retry := false + inProgress, err := ops.IsRebaseInProgress() + if err != nil { + return fmt.Errorf("checking rebase state: %w", err) + } + if inProgress { + if err := ops.RebaseContinue(git.RebaseOpts{CommitterDateIsAuthorDate: state.CommitterDateIsAuthorDate}); err != nil { + cfg.Errorf("rebase continue failed: %v", err) + printWorktreeConflict(cfg, state, state.RebaseBase) + return ErrConflict + } + } else { + sha, err := ops.RevParse(branch) + if err != nil { + return err + } + retry = state.Phase == "applying" && sha == ctx.PendingBefore + } + if retry { + opts := savedCascadeOpts(cfg, dir, state, s) + conflicted, err := rebaseStep(opts, branch, state.CurrentBranchIndex, state.RebaseBase, state.RebaseOldBase, state.RebaseOnto, state.UseOnto) + if err != nil { + cfg.Errorf("%s", err) + if conflicted { + printWorktreeConflict(cfg, state, state.RebaseBase) + return ErrConflict + } + return ErrSilent + } + } else { + containsBase, err := ops.IsAncestor(state.RebaseBase, branch) + if err != nil || !containsBase { + return fmt.Errorf("%s does not contain its saved rebase target; the Git rebase may have been aborted; run gh stack rebase --abort", branch) + } + if err := ctx.Record(branch); err != nil { + return err + } + state.ConflictBranch = "" + state.CurrentBranchIndex++ + state.Phase = "applying" + if err := saveRebaseState(dir, state); err != nil { + return err + } + } + cfg.Successf("Rebased %s", branch) + } + result := cascadeRebase(savedCascadeOpts(cfg, dir, state, s)) + if result.Err != nil { + cfg.Errorf("%s", result.Err) + if err := rollbackWorktreeRebase(cfg, dir, state); err != nil { + return errors.Join(ErrSilent, err) + } + return ErrSilent + } + if result.Conflicted { + printWorktreeConflict(cfg, state, result.ConflictBase) + return ErrConflict + } + } + if err := finishWorktreeRebase(cfg, dir, state, sf, s); err != nil { + return err + } + cfg.Successf("All branches in stack rebased locally") + cfg.Printf("To push your changes, run `%s`", cfg.ColorCyan("gh stack push")) + return nil +} + +func savedCascadeOpts(cfg *config.Config, dir string, state *rebaseState, s *stack.Stack) cascadeRebaseOpts { + return cascadeRebaseOpts{ + Cfg: cfg, + Stack: s, + Branches: s.Branches[state.CurrentBranchIndex:state.EndIndex], + StartAbsIdx: state.CurrentBranchIndex, + OriginalRefs: state.OriginalRefs, + NeedsOnto: state.UseOnto, + OntoOldBase: state.OntoOldBase, + CommitterDateIsAuthorDate: state.CommitterDateIsAuthorDate, + TrunkRef: state.TrunkRef, + TrunkSHA: state.TrunkSHA, + Worktrees: state.Worktrees, + State: state, + StateDir: dir, + } +} + +func finishWorktreeRebase(cfg *config.Config, dir string, state *rebaseState, sf *stack.StackFile, s *stack.Stack) error { + if err := state.Worktrees.RestoreOrigin(state.OriginalBranch); err != nil { + cfg.Errorf("%s", err) + return ErrSilent + } + target := state.TrunkSHA + if target == "" { + target = state.TrunkRef + } + if unstacked := verifyStacked(s, target, state.StartIndex, state.EndIndex); len(unstacked) > 0 { + reportUnstacked(cfg, state.TrunkRef, unstacked) + if err := rollbackWorktreeRebase(cfg, dir, state); err != nil { + return errors.Join(ErrSilent, err) + } + return ErrSilent + } + _ = syncStackPRs(cfg, s) + return publishCompletedRebase(cfg, dir, state, sf, s) +} + +func publishCompletedRebase(cfg *config.Config, dir string, state *rebaseState, sf *stack.StackFile, s *stack.Stack) error { + updateBaseSHAsWithTrunk(s, state.TrunkSHA) + state.Phase = "complete" + state.CurrentBranchIndex = state.EndIndex + if err := saveRebaseState(dir, state); err != nil { + cfg.Errorf("%s", err) + return ErrSilent + } + if err := stack.Save(dir, sf); err != nil { + return handleSaveError(cfg, err) + } + if err := clearRebaseState(dir); err != nil { + cfg.Errorf("rebase completed but recovery state could not be cleared: %v", err) + return ErrSilent + } + return nil +} diff --git a/cmd/sync.go b/cmd/sync.go index ff11626..d3800a1 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -63,12 +63,7 @@ than two PRs exist yet). Use --prune to delete local branches for merged PRs. Stack metadata is preserved so that rebase and display logic continue to work correctly. If you are on a branch that would be pruned, your checkout is moved to -the first active branch in the stack, or the trunk if all are merged. - -All stack branches and the trunk must currently be unoccupied or checked out -in this worktree. This includes remote-added branches and merged members. -Cross-worktree rewrites are refused before requested mutations, after -shared-catalog migration and any discovery fetches.`, +the first active branch in the stack, or the trunk if all are merged.`, RunE: func(cmd *cobra.Command, args []string) error { return runSync(cfg, opts) }, @@ -101,14 +96,6 @@ func runSync(cfg *config.Config, opts *syncOptions) error { s := result.Stack currentBranch := result.CurrentBranch originalTrunk := s.Trunk.Branch - if err := requireLocalBranches(cfg, s.BranchNames()); err != nil { - return err - } - ctx, err := worktree.New() - if err != nil { - cfg.Errorf("%s", err) - return ErrSilent - } // Resolve remote once for fetch and push remote, err := pickRemote(cfg, currentBranch, opts.remote) @@ -118,14 +105,6 @@ func runSync(cfg *config.Config, opts *syncOptions) error { } return ErrSilent } - trunkBranch, err := normalizeTrunkBranch(s.Trunk.Branch, remote) - if err != nil { - cfg.Errorf("%s", err) - return ErrSilent - } - if err := requireLocalBranches(cfg, []string{trunkBranch}); err != nil { - return err - } // --- Step 1: Fetch --- // Enable git rerere so conflict resolutions are remembered. @@ -171,22 +150,42 @@ func runSync(cfg *config.Config, opts *syncOptions) error { if cb, cbErr := git.CurrentBranch(); cbErr == nil { currentBranch = cb } - trunkBranch, err = normalizeTrunkBranch(s.Trunk.Branch, remote) + _ = syncStackPRs(cfg, s) + ctx, err := worktree.New() if err != nil { cfg.Errorf("%s", err) return ErrSilent } - if err := requireLocalBranches(cfg, append(s.BranchNames(), trunkBranch)); err != nil { - return err - } + planned := planFastForwardBranches(s, remote) + // --- Step 2: Resolve trunk --- - trunk, err := resolveTrunkTarget(cfg, s, remote, currentBranch) + trunk, err := resolveTrunkTarget(cfg, s, remote, currentBranch, trunkResolveOptions{ + Worktrees: ctx, + Preflight: func(sha string, moved bool) error { + var required []string + for _, forward := range planned { + required = append(required, forward.Branch) + } + if moved || len(planned) > 0 || stackNeedsRebase(s, sha) { + required = append(required, activeBranchNames(s)...) + } + if err := ctx.Preflight(required); err != nil { + cfg.Errorf("%s", err) + return ErrSilent + } + return nil + }, + }) if err != nil { return err } // --- Step 2b: Fast-forward stack branches behind their remote tracking branch --- - updatedBranches := fastForwardBranches(cfg, s, remote, currentBranch) + updatedBranches, err := fastForwardBranches(cfg, planned, ctx) + if err != nil { + cfg.Errorf("%s", err) + return ErrSilent + } // --- Step 3: Cascade rebase --- needsRebase := trunk.Moved || len(updatedBranches) > 0 || stackNeedsRebase(s, trunk.Ref) @@ -197,9 +196,6 @@ func runSync(cfg *config.Config, opts *syncOptions) error { cfg.Printf("") cfg.Printf("Rebasing stack ...") - // Sync PR state to detect merged PRs before rebasing. - _ = syncStackPRs(cfg, s) - originalRefs, err = resolveOriginalRefs(s) if err != nil { cfg.Errorf("Could not resolve branch SHAs: %v", err) @@ -222,12 +218,16 @@ func runSync(cfg *config.Config, opts *syncOptions) error { StartAbsIdx: 0, OriginalRefs: originalRefs, TrunkRef: trunk.Ref, + TrunkSHA: trunk.SHA, + Worktrees: ctx, + State: state, + StateDir: gitDir, }) if result.Err != nil { cfg.Errorf("%v", result.Err) - if err := abortRebase(cfg, gitDir); err != nil { - return err + if err := rollbackWorktreeRebase(cfg, gitDir, state); err != nil { + return errors.Join(ErrSilent, err) } return ErrSilent } @@ -235,11 +235,18 @@ func runSync(cfg *config.Config, opts *syncOptions) error { if result.Conflicted { // Abort and restore everything — sync is non-interactive. cfg.Errorf("Conflict detected rebasing %s onto %s", result.ConflictBranch, result.ConflictBase) - if err := abortRebase(cfg, gitDir); err != nil { - return err + if err := rollbackWorktreeRebase(cfg, gitDir, state); err != nil { + return errors.Join(ErrSilent, err) } + cfg.Printf("Branches restored to their pre-rebase state") cfg.Printf(" Run `%s` to resolve conflicts interactively.", cfg.ColorCyan("gh stack rebase")) + + // Persist refreshed PR state even on conflict, then bail out + // before pushing or reporting success. + if err := stack.Save(gitDir, sf); err != nil { + cfg.Warningf("Could not save refreshed PR metadata: %v", err) + } return ErrConflict } @@ -247,19 +254,23 @@ func runSync(cfg *config.Config, opts *syncOptions) error { rebased = true } } + if err := ctx.RestoreOrigin(currentBranch); err != nil { + cfg.Errorf("%s", err) + return ErrSilent + } } - if unstacked := verifyStacked(s, trunk.Ref, 0, len(s.Branches)); len(unstacked) > 0 { + if unstacked := verifyStacked(s, trunk.SHA, 0, len(s.Branches)); len(unstacked) > 0 { reportUnstacked(cfg, trunk.Ref, unstacked) if state != nil { - if err := abortRebase(cfg, gitDir); err != nil { - return err + if err := rollbackWorktreeRebase(cfg, gitDir, state); err != nil { + return errors.Join(ErrSilent, err) } } return ErrSilent } if state != nil { - if err := finishOriginRebase(cfg, gitDir, state, sf, s); err != nil { + if err := publishCompletedRebase(cfg, gitDir, state, sf, s); err != nil { return err } } @@ -399,14 +410,24 @@ func runSync(cfg *config.Config, opts *syncOptions) error { } } if needsSwitch { - switchTarget := trunk.Branch + switchTarget := "" for _, b := range s.Branches { if !b.IsSkipped() { + if owner := ctx.Owners[b.Branch]; owner != nil && !worktree.SamePath(owner.Path, ctx.Origin.Path) { + continue + } switchTarget = b.Branch break } } - if err := git.CheckoutBranch(switchTarget); err != nil { + if switchTarget == "" { + if owner := ctx.Owners[trunk.Branch]; owner == nil || worktree.SamePath(owner.Path, ctx.Origin.Path) { + switchTarget = trunk.Branch + } + } + if switchTarget == "" { + cfg.Infof("Keeping %s: no available checkout destination", currentBranch) + } else if err := git.CheckoutBranch(switchTarget); err != nil { cfg.Warningf("Failed to switch from %s to %s: %v", currentBranch, switchTarget, err) } else { currentBranch = switchTarget @@ -416,6 +437,14 @@ func runSync(cfg *config.Config, opts *syncOptions) error { cfg.Printf("") pruned := 0 for _, name := range prunable { + if owner := ctx.Owners[name]; owner != nil && !worktree.SamePath(owner.Path, ctx.Origin.Path) { + cfg.Infof("Keeping %s: checked out in worktree %s", name, owner.Path) + continue + } + if name == currentBranch { + cfg.Infof("Keeping %s: still checked out", name) + continue + } if err := git.DeleteBranch(name, true); err != nil { cfg.Warningf("Failed to delete %s: %v", name, err) } else { @@ -435,6 +464,9 @@ func runSync(cfg *config.Config, opts *syncOptions) error { // the local branch was already deleted. This prevents // `git checkout ` from resurrecting the branch. for _, b := range merged { + if owner := ctx.Owners[b.Branch]; owner != nil && !worktree.SamePath(owner.Path, ctx.Origin.Path) { + continue + } _ = git.DeleteTrackingRef(remote, b.Branch) } } @@ -469,15 +501,18 @@ func restoreBranches(originalRefs map[string]string) ([]string, error) { return nil, fmt.Errorf("checking branch %s before restoring: %w", branch, err) } if exists { - branches = append(branches, branch) + sha, err := git.RevParse(branch) + if err != nil { + return nil, fmt.Errorf("reading branch %s before restoring: %w", branch, err) + } + if sha != originalRefs[branch] { + branches = append(branches, branch) + } } } var errors []string for _, branch := range branches { sha := originalRefs[branch] - if currentSHA, err := git.RevParse(branch); err == nil && currentSHA == sha { - continue - } if err := git.CheckoutBranch(branch); err != nil { errors = append(errors, fmt.Sprintf("checkout %s: %s", branch, err)) continue @@ -493,10 +528,16 @@ func restoreRebaseRefs(cfg *config.Config, originalBranch string, originalRefs m restoreErrors, err := restoreBranches(originalRefs) if err != nil { cfg.Errorf("%s", err) - return ErrSilent + return errors.Join(ErrSilent, err) + } + checkoutErr := restoreRebaseCheckout(originalBranch) + if checkoutErr != nil { + restoreErrors = append(restoreErrors, checkoutErr.Error()) } - _ = git.CheckoutBranch(originalBranch) reportRestoreStatus(cfg, restoreErrors) + if len(restoreErrors) > 0 { + return errors.Join(ErrSilent, errors.New(strings.Join(restoreErrors, "\n")), checkoutErr) + } return nil } diff --git a/cmd/sync_test.go b/cmd/sync_test.go index 14b76b7..eb23322 100644 --- a/cmd/sync_test.go +++ b/cmd/sync_test.go @@ -509,6 +509,7 @@ func TestSync_RebaseConflict_RestoresAll(t *testing.T) { } mock := newSyncMock(tmpDir, "b1") + mock.CurrentBranchFn = func() (string, error) { return currentBranch, nil } mock.RevParseFn = func(ref string) (string, error) { if ref == "main" { return "local-sha", nil @@ -519,12 +520,21 @@ func TestSync_RebaseConflict_RestoresAll(t *testing.T) { if sha, ok := branchSHAs[ref]; ok { return sha, nil } + if sha, ok := branchSHAs[strings.TrimPrefix(ref, "origin/")]; ok { + return sha, nil + } return "sha-" + ref, nil } mock.IsAncestorFn = func(a, d string) (bool, error) { return true, nil } - mock.UpdateBranchRefFn = func(string, string) error { return nil } + mock.UpdateBranchRefFn = func(branch, sha string) error { + if _, ok := branchSHAs[branch]; ok { + resets = append(resets, resetCall{branch, sha}) + branchSHAs[branch] = sha + } + return nil + } mock.CheckoutBranchFn = func(name string) error { checkouts = append(checkouts, name) currentBranch = name @@ -782,7 +792,7 @@ func TestSync_MergedBranch_UsesOnto(t *testing.T) { // b2: first active branch after merged → RebaseOnto(main, b1-orig-sha, b2) // b3: normal --onto → RebaseOnto(b2, b2-orig-sha, b3) require.Len(t, rebaseOntoCalls, 2) - assert.Equal(t, rebaseCall{"main", "b1-orig-sha", "b2"}, rebaseOntoCalls[0]) + assert.Equal(t, rebaseCall{"remote-sha", "b1-orig-sha", "b2"}, rebaseOntoCalls[0]) assert.Equal(t, rebaseCall{"b2", "b2-orig-sha", "b3"}, rebaseOntoCalls[1]) // Push should use force (rebase happened) @@ -946,7 +956,7 @@ func TestSync_StaleOntoOldBase_UsesForkPoint(t *testing.T) { require.Len(t, rebaseOntoCalls, 2) // b2: stale ontoOldBase → uses fork-point(main, b2) - assert.Equal(t, rebaseCall{"main", "main-b2-forkpoint", "b2"}, rebaseOntoCalls[0], + assert.Equal(t, rebaseCall{"remote-sha", "main-b2-forkpoint", "b2"}, rebaseOntoCalls[0], "b2 should use the reflog fork-point when ontoOldBase is stale") // b3: b2's SHA is a valid ancestor → uses it directly @@ -1088,7 +1098,7 @@ func TestSync_BranchFastForward_TriggersRebase(t *testing.T) { // b1 should be fast-forwarded via MergeFF (since we're on b1) require.Len(t, mergeFFCalls, 1, "should fast-forward b1 via MergeFF") - assert.Equal(t, "origin/b1", mergeFFCalls[0]) + assert.Equal(t, "b1-remote-sha", mergeFFCalls[0]) assert.Contains(t, output, "Fast-forwarded b1") // Cascade rebase should be triggered (even though trunk didn't move) @@ -1266,7 +1276,7 @@ func TestSync_MergedBranchDeletedFromRemote(t *testing.T) { // Head SHA as oldBase so `git rebase --onto` receives valid arguments. require.Len(t, rebaseOntoCalls, 1) assert.Equal(t, "b2", rebaseOntoCalls[0].branch) - assert.Equal(t, "main", rebaseOntoCalls[0].newBase) + assert.Equal(t, "remote-sha", rebaseOntoCalls[0].newBase) assert.Equal(t, "b1-stored-head-sha", rebaseOntoCalls[0].oldBase) } @@ -2608,131 +2618,84 @@ func TestSync_MergedBranchPruned_NoFalseDivergence(t *testing.T) { assert.NotContains(t, output, "diverged") } -func TestSync_ForeignTargetsRefusedBeforeMutation(t *testing.T) { - for _, owner := range []string{"b1", "main"} { - t.Run(owner, func(t *testing.T) { - dir := t.TempDir() - writeStackFile(t, dir, stack.Stack{ - Trunk: stack.BranchRef{Branch: "main"}, - Branches: []stack.BranchRef{ - {Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 1, Merged: true}}, - {Branch: "b2"}, - }, - }) - before, err := os.ReadFile(filepath.Join(dir, "gh-stack")) - require.NoError(t, err) - mock := newSyncMock(dir, "b2") - mockForeignOwner(t, mock, dir, "b2", owner) - forbidRewriteMutations(t, mock) - output, err := runSyncCfg(t, mock, func(cfg *config.Config) { - cfg.GitHubClientOverride = &github.MockClient{} - }) - require.ErrorIs(t, err, ErrInvalidArgs) - assert.Contains(t, output, "cross-worktree rebase and sync are not supported yet") - after, err := os.ReadFile(filepath.Join(dir, "gh-stack")) - require.NoError(t, err) - assert.Equal(t, before, after) - assert.NoFileExists(t, filepath.Join(dir, rebaseStateFile)) - }) +func TestSync_WorktreesPublishesFromLinkedWorktree(t *testing.T) { + repo := setupWorktreeRebaseRepo(t, false) + withIssue250Repo(t, repo.childDir) + cfg := issue250TestConfig(t) + + require.NoError(t, runSync(cfg, &syncOptions{remote: "origin"})) + + assert.Equal(t, "main", issue250Git(t, repo.dir, "branch", "--show-current")) + assert.Equal(t, "parent", issue250Git(t, repo.parentDir, "branch", "--show-current")) + assert.Equal(t, "child", issue250Git(t, repo.childDir, "branch", "--show-current")) + for _, branch := range []string{"parent", "child"} { + assert.Equal(t, issue250Git(t, repo.dir, "rev-parse", branch), issue250Git(t, repo.dir, "rev-parse", "origin/"+branch)) } + require.NoError(t, issue250GitMayFail(t, repo.dir, "merge-base", "--is-ancestor", "parent", "child")) } -func TestSync_ForeignRemoteTargetsRefusedBeforeReconciliation(t *testing.T) { - for _, tc := range []struct { - name string - remote []int - choice int - duringPrompt bool - }{ - {name: "remote append", remote: []int{101, 102, 103}}, - {name: "replace local", remote: []int{101, 103}, choice: 0, duringPrompt: true}, - {name: "delete remote", remote: []int{101, 103}, choice: 1, duringPrompt: true}, - } { - t.Run(tc.name, func(t *testing.T) { - dir := t.TempDir() - writeStackFile(t, dir, stack.Stack{ - ID: "9", Number: 9, Trunk: stack.BranchRef{Branch: "main"}, - Branches: []stack.BranchRef{{Branch: "b1"}, {Branch: "b2"}}, - }) - before, err := os.ReadFile(filepath.Join(dir, "gh-stack")) - require.NoError(t, err) - mock := newSyncMock(dir, "b2") - owner := mockForeignOwner(t, mock, dir, "b2", "b3") - owned := !tc.duringPrompt - worktrees := mock.WorktreesFn - mock.WorktreesFn = func() ([]git.Worktree, error) { - trees, err := worktrees() - if !owned { - trees[1].Branch = "unrelated" - } - return trees, err - } - forbidRewriteMutations(t, mock) - fetches := 0 - mock.FetchBranchesFn = func(string, []string) error { fetches++; return nil } - lookups := 0 - ghMock := &github.MockClient{ - ListStacksFn: func() ([]github.RemoteStack, error) { - lookups++ - return []github.RemoteStack{{ID: 9, Number: 9, PullRequests: tc.remote}}, nil - }, - FindPRByNumberFn: prByNumberFinder(map[int]string{101: "b1", 102: "b2", 103: "b3"}), - UnstackFn: func(int) (*github.RemoteStack, bool, error) { - t.Fatal("must not mutate the remote stack") - return nil, false, nil - }, - } +func TestSync_SharedTrunkSelectionRestoresOrigin(t *testing.T) { + repo := setupSharedTrunkRebaseRepo(t, false) + observerHead := issue250Git(t, repo.childDir, "rev-parse", "HEAD") + withIssue250Repo(t, repo.dir) + cfg := issue250TestConfig(t) + cfg.ForceInteractive = true + cfg.SelectFn = func(_, _ string, _ []string) (int, error) { return 0, nil } + cfg.ConfirmFn = func(string, bool) (bool, error) { return false, nil } - output, err := runSyncCfg(t, mock, func(cfg *config.Config) { - cfg.GitHubClientOverride = ghMock - cfg.ForceInteractive = true - cfg.SelectFn = func(string, string, []string) (int, error) { - require.True(t, tc.duringPrompt, "ownership preflight must precede reconciliation choices") - owned = true - return tc.choice, nil - } - }) - - require.ErrorIs(t, err, ErrInvalidArgs) - assert.Positive(t, fetches, "discovery fetch is an allowed prerequisite") - assert.Positive(t, lookups) - assert.Contains(t, output, owner) - after, err := os.ReadFile(filepath.Join(dir, "gh-stack")) - require.NoError(t, err) - assert.Equal(t, before, after) - assert.NoFileExists(t, filepath.Join(dir, rebaseStateFile)) - }) - } + require.NoError(t, runSync(cfg, &syncOptions{remote: "origin"})) + + assert.Equal(t, "main", issue250Git(t, repo.dir, "branch", "--show-current")) + assert.Equal(t, "parent", issue250Git(t, repo.parentDir, "branch", "--show-current")) + assert.Equal(t, observerHead, issue250Git(t, repo.childDir, "rev-parse", "HEAD")) + assert.Equal(t, issue250Git(t, repo.dir, "rev-parse", "child"), issue250Git(t, repo.dir, "rev-parse", "origin/child")) } -func TestSync_InvalidOriginRefusesBeforeReconciliation(t *testing.T) { - dir := t.TempDir() - writeStackFile(t, dir, stack.Stack{ - ID: "9", Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "b1"}}, - }) - mock := newSyncMock(dir, "b1") - mock.RootDirFn = func() (string, error) { return "", errors.New("origin unavailable") } - forbidRewriteMutations(t, mock) +func TestSync_WorktreesConflictRestoresWithoutPush(t *testing.T) { + repo := setupWorktreeRebaseRepo(t, true) + beforeParent := issue250Git(t, repo.dir, "rev-parse", "parent") + beforeChild := issue250Git(t, repo.dir, "rev-parse", "child") + remoteParent := issue250Git(t, repo.dir, "rev-parse", "origin/parent") + remoteChild := issue250Git(t, repo.dir, "rev-parse", "origin/child") + withIssue250Repo(t, repo.dir) + cfg := issue250TestConfig(t) - output, err := runSyncCfg(t, mock, func(cfg *config.Config) { - cfg.GitHubClientOverride = &github.MockClient{ListStacksFn: func() ([]github.RemoteStack, error) { - t.Fatal("origin validation must precede remote reconciliation") - return nil, nil - }} - }) + require.ErrorIs(t, runSync(cfg, &syncOptions{remote: "origin"}), ErrConflict) + + assert.Equal(t, beforeParent, issue250Git(t, repo.dir, "rev-parse", "parent")) + assert.Equal(t, beforeChild, issue250Git(t, repo.dir, "rev-parse", "child")) + assert.Equal(t, remoteParent, issue250Git(t, repo.dir, "rev-parse", "origin/parent")) + assert.Equal(t, remoteChild, issue250Git(t, repo.dir, "rev-parse", "origin/child")) + assert.False(t, requireGitState(t, requireWorktree(t, git.CurrentOps(), repo.childDir).IsRebaseInProgress)) + assert.Equal(t, "main", issue250Git(t, repo.dir, "branch", "--show-current")) + _, err := os.Stat(filepath.Join(repo.gitDir, rebaseStateFile)) + assert.ErrorIs(t, err, os.ErrNotExist) +} - require.ErrorIs(t, err, ErrSilent) - assert.Contains(t, output, "origin unavailable") +func TestSync_WorktreesPruneKeepsOccupiedMergedBranch(t *testing.T) { + repo := setupWorktreeRebaseRepo(t, false) + sf, err := stack.Load(repo.gitDir) + require.NoError(t, err) + sf.Stacks[0].Branches[0].PullRequest = &stack.PullRequestRef{Number: 101, Merged: true} + require.NoError(t, stack.Save(repo.gitDir, sf)) + withIssue250Repo(t, repo.childDir) + cfg := issue250TestConfig(t) + + require.NoError(t, runSync(cfg, &syncOptions{remote: "origin", prune: true})) + + assert.Equal(t, "parent", issue250Git(t, repo.parentDir, "branch", "--show-current")) + require.NoError(t, issue250GitMayFail(t, repo.dir, "show-ref", "--verify", "refs/heads/parent")) + assert.DirExists(t, repo.parentDir) } -func TestSync_RollbackFailureRetainsOriginRecovery(t *testing.T) { +func TestSync_RollbackFailureRetainsRecovery(t *testing.T) { dir := t.TempDir() writeStackFile(t, dir, stack.Stack{ Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "b1"}, {Branch: "b2"}}, }) current := "b2" refs := map[string]string{"main": "trunk", "b1": "old-b1", "b2": "old-b2"} - busy, failReset := false, true + busy, failRestore := false, true mock := newSyncMock(dir, current) mock.CurrentBranchFn = func() (string, error) { return current, nil } mock.RevParseFn = func(ref string) (string, error) { return refs[strings.TrimPrefix(ref, "origin/")], nil } @@ -2745,11 +2708,11 @@ func TestSync_RollbackFailureRetainsOriginRecovery(t *testing.T) { } mock.IsRebaseInProgressFn = func() (bool, error) { return busy, nil } mock.RebaseAbortFn = func() error { busy = false; return nil } - mock.ResetHardFn = func(sha string) error { - if failReset { - return errors.New("reset failed") + mock.UpdateBranchRefFn = func(branch, sha string) error { + if failRestore { + return errors.New("ref restore failed") } - refs[current] = sha + refs[branch] = sha return nil } mock.PushFn = func(string, []string, bool, bool) error { t.Fatal("must not push after failed rollback"); return nil } @@ -2761,11 +2724,13 @@ func TestSync_RollbackFailureRetainsOriginRecovery(t *testing.T) { state, err := loadRebaseState(dir) require.NoError(t, err) assert.Equal(t, "restoring", state.Phase) - assert.Equal(t, originOnlyRebaseMode, state.ExecutionMode) + assert.Empty(t, state.ExecutionMode) require.NotNil(t, state.Worktrees) + assert.Equal(t, map[string]string{"b1": "rebased-b1"}, state.Worktrees.Touched) + assert.Empty(t, state.Worktrees.Pending) assert.Equal(t, "rebased-b1", refs["b1"]) - failReset = false + failRestore = false require.NoError(t, runRebase(cfg, &rebaseOptions{abort: true})) assert.Equal(t, "old-b1", refs["b1"]) assert.Equal(t, "b2", current) diff --git a/cmd/trunk_target_test.go b/cmd/trunk_target_test.go index 7b91f56..b39678c 100644 --- a/cmd/trunk_target_test.go +++ b/cmd/trunk_target_test.go @@ -277,6 +277,12 @@ func TestRebase_LaterStartErrorRestoresEarlierBranches(t *testing.T) { var resets []resetCall mock := newRebaseMock(tmpDir, currentBranch) + mock.CurrentBranchFn = func() (string, error) { return currentBranch, nil } + mock.UpdateBranchRefFn = func(branch, sha string) error { + resets = append(resets, resetCall{branch, sha}) + branchSHAs[branch] = sha + return nil + } mock.BranchExistsFn = func(string) (bool, error) { return true, nil } mock.RevParseFn = func(ref string) (string, error) { if ref == "main" || ref == "origin/main" { @@ -336,6 +342,11 @@ func TestSync_LaterStartErrorRestoresEarlierBranches(t *testing.T) { pushes := 0 mock := newSyncMock(tmpDir, currentBranch) + mock.CurrentBranchFn = func() (string, error) { return currentBranch, nil } + mock.UpdateBranchRefFn = func(branch, sha string) error { + branchSHAs[branch] = sha + return nil + } mock.RevParseFn = func(ref string) (string, error) { if ref == "main" || ref == "origin/main" { return "trunk", nil @@ -499,7 +510,7 @@ func TestSync_UnstackedCascadeDoesNotPush(t *testing.T) { if a == "local" && d == "remote" { return true, nil } - if a == "main" && d == "b1" { + if (a == "main" || a == "remote") && d == "b1" { return false, nil } return true, nil diff --git a/cmd/utils.go b/cmd/utils.go index ab24d90..1f96392 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -17,6 +17,7 @@ import ( "github.com/github/gh-stack/internal/github" "github.com/github/gh-stack/internal/stack" "github.com/github/gh-stack/internal/theme" + "github.com/github/gh-stack/internal/worktree" ) // ErrSilent indicates the error has already been printed to the user. @@ -406,8 +407,9 @@ func handleSaveError(cfg *config.Config, err error) error { // resolveStack finds the stack for the given branch, handling ambiguity when // a branch (typically a trunk) belongs to multiple stacks. If exactly one // stack matches, it is returned directly. If multiple stacks match, the user -// is prompted to select one. Commands that need rewrite preflight and read-only -// callers leave checkout unchanged. Returns nil if no stack contains the branch. +// is prompted to select one. Read-only and rewrite callers preserve their +// checkout; other mutating callers switch to the selected stack's top branch. +// Returns nil with no error if no stack contains the branch. func resolveStack(sf *stack.StackFile, branch string, cfg *config.Config) (*stack.Stack, error) { stacks := sf.FindAllStacksForBranch(branch) @@ -454,12 +456,9 @@ func resolveStack(sf *stack.StackFile, branch string, cfg *config.Config) (*stac if len(s.Branches) == 0 { return nil, fmt.Errorf("selected stack %q has no branches", s.DisplayChain()) } - // Selection must not mutate a checkout before rewrite ownership preflight. - if cfg.StackMutation == nil { - return s, nil - } - switch cfg.StackMutation.Kind { - case "rebase", "rebase-continue", "rebase-abort", "sync", "modify": + // Selection must not change a rewrite's origin or bypass its preflight. + // Read-only selection also remains available while an operation is paused. + if cfg.StackMutation == nil || cfg.StackMutation.NoCheckoutOnSelect { return s, nil } @@ -886,8 +885,15 @@ func activeBranchNames(s *stack.Stack) []string { // tracking branch when the local branch is strictly behind. Returns the names // of branches that were updated. Branches that are up-to-date, diverged, or // have no remote tracking branch are silently skipped. -func fastForwardBranches(cfg *config.Config, s *stack.Stack, remote, currentBranch string) []string { - var updated []string +type branchFastForward struct { + Branch string + RemoteRef string + OldSHA string + NewSHA string +} + +func planFastForwardBranches(s *stack.Stack, remote string) []branchFastForward { + var planned []branchFastForward for _, br := range s.Branches { if br.IsSkipped() { continue @@ -912,23 +918,40 @@ func fastForwardBranches(cfg *config.Config, s *stack.Stack, remote, currentBran continue } - // Local is behind remote — fast-forward. - if currentBranch == br.Branch { - if err := git.MergeFF(remoteRef); err != nil { - cfg.Warningf("Failed to fast-forward %s from remote: %v", br.Branch, err) - continue + planned = append(planned, branchFastForward{br.Branch, remoteRef, localSHA, remoteSHA}) + } + return planned +} + +func fastForwardBranches(cfg *config.Config, planned []branchFastForward, ctx *worktree.Context) ([]string, error) { + var updated []string + for _, plan := range planned { + ops, err := ctx.Ops(plan.Branch) + if err != nil { + return updated, err + } + if sha, err := ops.RevParse(plan.Branch); err != nil || sha != plan.OldSHA { + return updated, fmt.Errorf("%s changed since fast-forward preflight; retry the command", plan.Branch) + } + current, err := ops.CurrentBranch() + if err != nil { + return updated, err + } + if current == plan.Branch { + if err := worktree.CheckClean(ops, ctx.Location(plan.Branch).Path); err != nil { + return updated, err } + err = ops.MergeFF(plan.NewSHA) } else { - if err := git.UpdateBranchRef(br.Branch, remoteSHA); err != nil { - cfg.Warningf("Failed to fast-forward %s from remote: %v", br.Branch, err) - continue - } + err = ops.UpdateBranchRef(plan.Branch, plan.NewSHA) } - - cfg.Successf("Fast-forwarded %s to %s", br.Branch, short(remoteSHA)) - updated = append(updated, br.Branch) + if err != nil { + return updated, fmt.Errorf("fast-forwarding %s: %w", plan.Branch, err) + } + cfg.Successf("Fast-forwarded %s to %s", plan.Branch, short(plan.NewSHA)) + updated = append(updated, plan.Branch) } - return updated + return updated, nil } // resolveOriginalRefs builds a map from branch name to current SHA for all @@ -1028,6 +1051,11 @@ type trunkTarget struct { Moved bool } +type trunkResolveOptions struct { + Worktrees *worktree.Context + Preflight func(string, bool) error +} + func (t trunkTarget) Describe() string { return fmt.Sprintf("%s (%s)", t.Ref, short(t.SHA)) } @@ -1035,7 +1063,19 @@ func (t trunkTarget) Describe() string { // resolveTrunkTarget fetches the trunk explicitly, then returns the ref the // cascade must use. Updating the local trunk is best-effort; the fetched remote // ref remains the source of truth when the local branch is stale or immovable. -func resolveTrunkTarget(cfg *config.Config, s *stack.Stack, remote, currentBranch string) (trunkTarget, error) { +func resolveTrunkTarget(cfg *config.Config, s *stack.Stack, remote, currentBranch string, options ...trunkResolveOptions) (trunkTarget, error) { + var opts trunkResolveOptions + if len(options) > 0 { + opts = options[0] + } + checked := false + preflight := func(sha string, moved bool) error { + if checked || opts.Preflight == nil { + return nil + } + checked = true + return opts.Preflight(sha, moved) + } if err := normalizeStackTrunk(cfg, s, remote); err != nil { return trunkTarget{}, err } @@ -1044,7 +1084,11 @@ func resolveTrunkTarget(cfg *config.Config, s *stack.Stack, remote, currentBranc if err := git.FetchBranch(remote, trunk); err != nil { if errors.Is(err, git.ErrRemoteBranchNotFound) { - return trunkWithoutRemote(cfg, trunk, remote) + target, err := trunkWithoutRemote(cfg, trunk, remote) + if err == nil { + err = preflight(target.SHA, target.Moved) + } + return target, err } cfg.Errorf("failed to fetch trunk branch %s from %s: %v", trunk, remote, err) return trunkTarget{}, ErrSilent @@ -1062,6 +1106,9 @@ func resolveTrunkTarget(cfg *config.Config, s *stack.Stack, remote, currentBranc return trunkTarget{}, fmt.Errorf("checking trunk branch %s: %w", trunk, err) } if !exists { + if err := preflight(remoteSHA, true); err != nil { + return trunkTarget{}, err + } if err := git.CreateBranch(trunk, remoteRef); err != nil { cfg.Errorf("could not create local trunk branch %s from %s: %v", trunk, remoteRef, err) return trunkTarget{}, ErrSilent @@ -1076,17 +1123,35 @@ func resolveTrunkTarget(cfg *config.Config, s *stack.Stack, remote, currentBranc return trunkTarget{}, ErrSilent } if localSHA == remoteSHA { + if err := preflight(localSHA, false); err != nil { + return trunkTarget{}, err + } cfg.Successf("Trunk %s is already up to date", trunk) return trunkTarget{Branch: trunk, Ref: trunk, SHA: localSHA}, nil } canFastForward, ffErr := git.IsAncestor(localSHA, remoteSHA) if ffErr == nil && canFastForward { + if err := preflight(remoteSHA, true); err != nil { + return trunkTarget{}, err + } var updateErr error - if currentBranch == trunk { - updateErr = git.MergeFF(remoteRef) - } else { - updateErr = git.UpdateBranchRef(trunk, remoteSHA) + ops := git.CurrentOps() + if opts.Worktrees != nil { + ops, updateErr = opts.Worktrees.Ops(trunk) + if updateErr == nil { + currentBranch, updateErr = ops.CurrentBranch() + if updateErr == nil && currentBranch == trunk { + updateErr = worktree.CheckClean(ops, opts.Worktrees.Location(trunk).Path) + } + } + } + if updateErr == nil { + if currentBranch == trunk { + updateErr = ops.MergeFF(remoteRef) + } else { + updateErr = ops.UpdateBranchRef(trunk, remoteSHA) + } } if updateErr == nil { cfg.Successf("Trunk %s fast-forwarded to %s", trunk, short(remoteSHA)) @@ -1098,12 +1163,18 @@ func resolveTrunkTarget(cfg *config.Config, s *stack.Stack, remote, currentBranc } else if isAncestor, ancErr := git.IsAncestor(remoteSHA, localSHA); ancErr == nil && isAncestor { // Keep unpushed local trunk commits when they already contain the // fetched remote tip. + if err := preflight(localSHA, false); err != nil { + return trunkTarget{}, err + } cfg.Successf("Trunk %s is ahead of %s — using the local branch", trunk, remoteRef) return trunkTarget{Branch: trunk, Ref: trunk, SHA: localSHA}, nil } else { cfg.Warningf("Local %s has diverged from %s", trunk, remoteRef) } + if err := preflight(remoteSHA, false); err != nil { + return trunkTarget{}, err + } cfg.Printf(" Rebasing the stack onto %s instead; local %s is unchanged.", remoteRef, trunk) return trunkTarget{Branch: trunk, Ref: remoteRef, SHA: remoteSHA}, nil } @@ -1145,6 +1216,10 @@ type cascadeRebaseOpts struct { OntoOldBase string CommitterDateIsAuthorDate bool TrunkRef string + TrunkSHA string + Worktrees *worktree.Context + State *rebaseState + StateDir string } func (o cascadeRebaseOpts) trunkRef() string { @@ -1196,6 +1271,76 @@ type cascadeRebaseResult struct { OntoOldBase string // ontoOldBase at the conflict point (for --continue) } +func rebaseStep(opts cascadeRebaseOpts, branch string, absIdx int, base, oldBase string, useOnto, needsOnto bool) (bool, error) { + executionBase := base + if opts.TrunkSHA != "" && base == opts.trunkRef() { + executionBase = opts.TrunkSHA + } + if opts.Worktrees != nil { + if err := opts.Worktrees.Start(branch, opts.OriginalRefs[branch]); err != nil { + return false, err + } + } + if state := opts.State; state != nil { + state.Phase = "applying" + state.CurrentBranchIndex = absIdx + state.ConflictBranch = branch + state.RemainingBranches = nil + for _, remaining := range opts.Branches { + if opts.Stack.IndexOf(remaining.Branch) > absIdx { + state.RemainingBranches = append(state.RemainingBranches, remaining.Branch) + } + } + state.UseOnto = needsOnto + state.OntoOldBase = opts.OriginalRefs[branch] + state.RebaseBase = executionBase + state.RebaseOldBase = oldBase + state.RebaseOnto = useOnto + if err := saveRebaseState(opts.StateDir, state); err != nil { + return false, err + } + } + ops := git.CurrentOps() + var err error + if opts.Worktrees != nil { + ops, err = opts.Worktrees.Prepare(branch) + } else if !useOnto { + err = ops.CheckoutBranch(branch) + } + if err != nil { + return false, fmt.Errorf("preparing %s: %w", branch, err) + } + rebaseOpts := git.RebaseOpts{CommitterDateIsAuthorDate: opts.CommitterDateIsAuthorDate} + if useOnto { + err = ops.RebaseOnto(executionBase, oldBase, branch, rebaseOpts) + } else { + err = ops.Rebase(executionBase, rebaseOpts) + } + if err != nil { + conflicted := !git.IsRebaseStartError(err) + if conflicted && opts.State != nil { + opts.State.Phase = "conflict" + if saveErr := saveRebaseState(opts.StateDir, opts.State); saveErr != nil { + return false, errors.Join(err, saveErr) + } + } + return conflicted, err + } + if opts.Worktrees != nil { + if err := opts.Worktrees.Record(branch); err != nil { + return false, fmt.Errorf("recording completed rebase of %s: %w", branch, err) + } + } + if opts.State != nil { + opts.State.CurrentBranchIndex = absIdx + 1 + opts.State.ConflictBranch = "" + if err := saveRebaseState(opts.StateDir, opts.State); err != nil { + return false, err + } + } + return false, nil +} + // cascadeRebase performs a cascade rebase across the given branch range. It // stops at the first conflict and returns a result describing what happened. // The caller is responsible for conflict recovery (abort+restore or save state). @@ -1206,7 +1351,6 @@ func cascadeRebase(opts cascadeRebaseOpts) cascadeRebaseResult { ontoOldBase := opts.OntoOldBase originalRefs := opts.OriginalRefs result := cascadeRebaseResult{} - rebaseOpts := git.RebaseOpts{CommitterDateIsAuthorDate: opts.CommitterDateIsAuthorDate} trunkRef := opts.trunkRef() for i, br := range opts.Branches { @@ -1259,8 +1403,8 @@ func cascadeRebase(opts cascadeRebaseOpts) cascadeRebaseResult { } } - if err := git.RebaseOnto(newBase, actualOldBase, br.Branch, rebaseOpts); err != nil { - if git.IsRebaseStartError(err) { + if conflicted, err := rebaseStep(opts, br.Branch, absIdx, newBase, actualOldBase, true, true); err != nil { + if !conflicted { return cascadeRebaseResult{ Rebased: result.Rebased, Err: fmt.Errorf("could not start rebase of %s onto %s: %w", br.Branch, newBase, err), @@ -1287,6 +1431,7 @@ func cascadeRebase(opts cascadeRebaseOpts) cascadeRebaseResult { ontoOldBase = originalRefs[br.Branch] } else { var rebaseErr error + var conflicted bool if absIdx > 0 { oldBase, err := resolveRebaseOldBase(originalRefs[base], br.Base, base, br.Branch) if err != nil { @@ -1295,19 +1440,13 @@ func cascadeRebase(opts cascadeRebaseOpts) cascadeRebaseResult { Err: err, } } - rebaseErr = git.RebaseOnto(base, oldBase, br.Branch, rebaseOpts) + conflicted, rebaseErr = rebaseStep(opts, br.Branch, absIdx, base, oldBase, true, false) } else { - if err := git.CheckoutBranch(br.Branch); err != nil { - return cascadeRebaseResult{ - Rebased: result.Rebased, - Err: fmt.Errorf("checking out %s: %w", br.Branch, err), - } - } - rebaseErr = git.Rebase(base, rebaseOpts) + conflicted, rebaseErr = rebaseStep(opts, br.Branch, absIdx, base, "", false, false) } if rebaseErr != nil { - if git.IsRebaseStartError(rebaseErr) { + if !conflicted { return cascadeRebaseResult{ Rebased: result.Rebased, Err: fmt.Errorf("could not start rebase of %s onto %s: %w", br.Branch, base, rebaseErr), @@ -1670,9 +1809,6 @@ func reconcileRemoteStack(cfg *config.Config, sf *stack.StackFile, s *stack.Stac if err != nil { return res, nil } - if err := preflightSyncReconciliation(cfg, s, prs, remote); err != nil { - return res, err - } localActive, remoteActive := activeStackSequences(s, prs) @@ -1691,22 +1827,6 @@ func reconcileRemoteStack(cfg *config.Config, sf *stack.StackFile, s *stack.Stac } } -func preflightSyncReconciliation(cfg *config.Config, s *stack.Stack, prs []*github.PullRequest, remote string) error { - if cfg.StackMutation == nil || cfg.StackMutation.Kind != "sync" { - return nil - } - trunk, err := normalizeTrunkBranch(s.Trunk.Branch, remote) - if err != nil { - cfg.Errorf("%s", err) - return ErrSilent - } - branches := append(s.BranchNames(), trunk) - for _, pr := range prs { - branches = append(branches, pr.HeadRefName) - } - return requireLocalBranches(cfg, branches) -} - // activeStackSequences returns the ordered active (non-merged) branch-name // sequences for the local stack and the fetched remote PRs. Merged state is // taken from the freshly fetched remote PRs (by branch name) when available so @@ -1901,11 +2021,6 @@ func resolveStackDivergence(cfg *config.Config, client github.ClientOps, sf *sta cfg.Errorf("selection failed: %v", err) return remoteReconcileResult{}, ErrSilent } - if selected == 0 || selected == 1 { - if err := preflightSyncReconciliation(cfg, s, prs, remote); err != nil { - return remoteReconcileResult{}, err - } - } switch selected { case 0: diff --git a/cmd/utils_test.go b/cmd/utils_test.go index f558a56..475e69c 100644 --- a/cmd/utils_test.go +++ b/cmd/utils_test.go @@ -78,6 +78,86 @@ func TestResolveStack_ReadOnlySelectionDoesNotCheckout(t *testing.T) { commandOutput(t, cfg, outR, errR) } +func TestResolveStack_RewriteSelectionPreservesCheckout(t *testing.T) { + for _, kind := range []string{"rebase", "rebase-continue", "rebase-abort", "sync", "modify", "modify-continue", "modify-abort", "add", "checkout", "submit"} { + t.Run(kind, func(t *testing.T) { + dir := t.TempDir() + writeStackFileMulti(t, dir, + stack.Stack{Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "b1"}, {Branch: "b2"}}}, + stack.Stack{Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "independent"}}}, + ) + current := "main" + var checkouts []string + mock := newRebaseMock(dir, current) + mock.CurrentBranchFn = func() (string, error) { return current, nil } + mock.CheckoutBranchFn = func(branch string) error { + checkouts = append(checkouts, branch) + current = branch + return nil + } + restore := git.SetOps(mock) + defer restore() + cfg := issue250TestConfig(t) + cfg.ForceInteractive = true + cfg.SelectFn = func(_, _ string, choices []string) (int, error) { + require.Len(t, choices, 2) + return 0, nil + } + release, err := beginStackMutation(cfg, kind) + require.NoError(t, err) + defer release() + + result, err := loadStackOptional(cfg, "") + + require.NoError(t, err) + assert.Equal(t, []string{"b1", "b2"}, result.Stack.BranchNames()) + if kind == "add" || kind == "checkout" || kind == "submit" { + assert.Equal(t, []string{"b2"}, checkouts, "intentional selection checkout must remain available") + assert.Equal(t, "b2", result.CurrentBranch) + } else { + assert.Empty(t, checkouts, "rewrite selection must not move the origin before preflight") + assert.Equal(t, "main", result.CurrentBranch) + } + assert.Equal(t, result.CurrentBranch, current) + }) + } +} + +func TestStackSelection_RewritePreflightPreservesCheckout(t *testing.T) { + for _, command := range []struct { + name string + run func(*config.Config) error + }{ + {"rebase", func(cfg *config.Config) error { return runRebase(cfg, &rebaseOptions{remote: "origin"}) }}, + {"sync", func(cfg *config.Config) error { return runSync(cfg, &syncOptions{remote: "origin"}) }}, + {"modify", runModify}, + } { + t.Run(command.name, func(t *testing.T) { + repo := setupSharedTrunkRebaseRepo(t, false) + issue250WriteFile(t, repo.parentDir, "unfinished.txt", "preserve this work\n") + beforeRefs := issue250Git(t, repo.dir, "show-ref") + beforeCatalog, err := os.ReadFile(filepath.Join(repo.gitDir, "gh-stack")) + require.NoError(t, err) + withIssue250Repo(t, repo.dir) + cfg := issue250TestConfig(t) + cfg.ForceInteractive = true + cfg.SelectFn = func(_, _ string, _ []string) (int, error) { return 0, nil } + cfg.ConfirmFn = func(string, bool) (bool, error) { return false, nil } + + require.Error(t, command.run(cfg)) + + assert.Equal(t, "main", issue250Git(t, repo.dir, "branch", "--show-current")) + assert.Equal(t, "parent", issue250Git(t, repo.parentDir, "branch", "--show-current")) + assert.Equal(t, beforeRefs, issue250Git(t, repo.dir, "show-ref")) + afterCatalog, err := os.ReadFile(filepath.Join(repo.gitDir, "gh-stack")) + require.NoError(t, err) + assert.Equal(t, beforeCatalog, afterCatalog) + assert.FileExists(t, filepath.Join(repo.parentDir, "unfinished.txt")) + assert.NoFileExists(t, filepath.Join(repo.gitDir, rebaseStateFile)) + }) + } +} + func TestStackMutation_NestedAndReadOnly(t *testing.T) { common := t.TempDir() restore := git.SetOps(&git.MockOps{GitDirFn: func() (string, error) { return common, nil }}) diff --git a/cmd/worktree_utils.go b/cmd/worktree_utils.go index a2733a4..a96e56a 100644 --- a/cmd/worktree_utils.go +++ b/cmd/worktree_utils.go @@ -77,7 +77,11 @@ func beginStackMutation(cfg *config.Config, kind string) (func(), error) { lock.Unlock() return nil, err } - cfg.StackMutation = &config.StackMutationContext{CommonDir: commonDir, StateDir: stateDir, Kind: kind} + cfg.StackMutation = &config.StackMutationContext{CommonDir: commonDir, StateDir: stateDir} + switch kind { + case "rebase", "rebase-continue", "rebase-abort", "sync", "modify", "modify-continue", "modify-abort": + cfg.StackMutation.NoCheckoutOnSelect = true + } released := false return func() { if !released { @@ -166,6 +170,9 @@ func readStackJournals(cfg *config.Config, commonDir, localDir string) ([]stackJ if err == nil && state == nil { err = fmt.Errorf("invalid rebase recovery record") } + if err == nil { + err = validateRebaseExecutionMode(state) + } if err == nil { journal.phase, journal.legacy = state.Phase, state.Worktrees == nil if state.Worktrees != nil { @@ -385,29 +392,6 @@ func foreignWorktreePath(target string) (string, error) { return owner, nil } -// Until owner-scoped rebase/sync execution is enabled, include every possible -// write and rollback target, not just the requested rebase range. -func requireLocalBranches(cfg *config.Config, branches []string) error { - seen := make(map[string]bool) - for _, branch := range branches { - if branch == "" || seen[branch] { - continue - } - seen[branch] = true - owner, err := foreignWorktreePath(branch) - if err != nil { - cfg.Errorf("%s", err) - return ErrSilent - } - if owner != "" { - reportWorktreeOwner(cfg, branch, owner) - cfg.Errorf("cross-worktree rebase and sync are not supported yet; all affected branches must be unoccupied or checked out in this worktree") - return ErrInvalidArgs - } - } - return nil -} - func reportWorktreeOwner(cfg *config.Config, target, path string) { commandPath := path if strings.ContainsFunc(path, func(r rune) bool { diff --git a/docs/src/content/docs/getting-started/quick-start.md b/docs/src/content/docs/getting-started/quick-start.md index 111f0fa..9c7af72 100644 --- a/docs/src/content/docs/getting-started/quick-start.md +++ b/docs/src/content/docs/getting-started/quick-start.md @@ -97,7 +97,7 @@ This shows all branches, their PR links, statuses, and the most recent commit on Linked worktrees share the same local stack catalog. You can adopt branches already checked out elsewhere with `gh stack init branch-a branch-b` or `gh stack add branch-c`; adoption does not move either checkout. `add`'s commit/stage shortcuts cannot target another worktree. -`rebase` and `sync` currently require all stack branches and any trunk they update to be unoccupied or checked out in the invoking worktree. They refuse distributed rewrites rather than skipping layers. 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). +`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. diff --git a/docs/src/content/docs/guides/workflows.md b/docs/src/content/docs/guides/workflows.md index 32a8133..7c16c83 100644 --- a/docs/src/content/docs/guides/workflows.md +++ b/docs/src/content/docs/guides/workflows.md @@ -15,8 +15,6 @@ The catalog is `/gh-stack`, where the common directory is reported b On upgrade, gh-stack automatically consolidates nonconflicting legacy catalogs, coalesces equivalent definitions, and preserves originals as backups. Sharing a trunk is fine; conflicting branch membership or stack definitions stop migration and identify the source files. Reconcile the conflicting definitions rather than deleting whichever file looks older. Finish or abort legacy operations in their original worktree before migration, and do not run old and new gh-stack versions against the same clone. -Migration is a prerequisite to the requested operation. It may publish the shared catalog and preservation backups even when the operation later refuses a foreign-owned rewrite. After migration, that refusal leaves refs, indexes, working files, requested stack membership, and remote stacks unchanged. Fetches used for discovery may still have completed. - ### 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. @@ -67,12 +65,16 @@ gh-stack does not install shell functions or change your shell's directory. Do n ### Rebase, sync, and recover -`rebase` and `sync` currently use only the initiating worktree. All stack members must be unoccupied or checked out there, including members outside a requested rebase range and merged members that rollback or pruning could touch. A foreign-owned trunk is refused when trunk updates are enabled; `rebase --no-trunk` leaves it alone. Sync checks remote-added and replacement branches before importing them. This conservative limit prevents partial distributed rewrites rather than silently skipping layers. gh-stack never auto-stashes, transfers ownership, or creates/removes worktrees. +`rebase` and `sync` automatically operate in each affected branch's clean owning worktree. Unoccupied branches are processed in the initiating worktree, whose original checkout is restored afterward. Dirty, busy, missing, or changed affected owners block mutation; unrelated worktrees are left alone. A clean trunk owner can be fast-forwarded, while an unsafe local trunk retains the fetched-remote fallback. gh-stack never auto-stashes, transfers ownership, or creates/removes worktrees. + +When a trunk belongs to multiple stacks, selecting one for `rebase`, `sync`, or `modify` does not switch branches. Rebase ranges use the caller's original checkout unless an explicit branch is supplied. -Resolve and stage conflicts in the worktree named by the diagnostic, then run `gh stack rebase --continue` or `--abort` **in that original worktree**. Invoking rebase recovery elsewhere fails before Git mutation. `sync` restores its cascade on conflicts rather than pushing partial results; completed fetches and earlier fast-forwards are outside that rollback boundary. Partial restoration or publication failures retain recovery state. Repair the reported problem and retry recovery in the origin rather than deleting the journal. +Resolve and stage conflicts in the worktree named by the diagnostic. You can run `gh stack rebase --continue` or `--abort` from any linked worktree: the shared journal routes recovery to the recorded owners. `sync` still restores its cascade on conflicts rather than pushing partial results; completed fetches and earlier fast-forwards are outside that rollback boundary. Recovery retains state and reports any partial failure rather than discarding later edits or claiming a full restoration. Pruning skips branches still occupied in other worktrees. Finish paused operations before changing gh-stack versions or preview stages. Origin-only journals are explicitly marked; a build that cannot interpret a journal's execution lifecycle must leave it intact. If recovery reports an incompatible lifecycle, use the matching build in the recorded origin to finish or abort it instead of editing or removing the journal. +Legacy recovery remains in its original worktree. Interrupted application or restoration must be aborted; a completed operation retries only its original checkout and catalog publication. Failed restoration, checkout, or publication retains the journal for recovery rather than replaying completed work. + 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/). diff --git a/docs/src/content/docs/reference/cli.md b/docs/src/content/docs/reference/cli.md index 961b851..2d8ea59 100644 --- a/docs/src/content/docs/reference/cli.md +++ b/docs/src/content/docs/reference/cli.md @@ -19,7 +19,7 @@ The `gh stack` CLI uses your GitHub CLI authentication — run `gh auth login` i All linked worktrees share `/gh-stack` and gh-stack recovery journals. Native Git HEAD, index, rebase, and cherry-pick markers remain per-worktree. Mutations are serialized across the clone; read-only views remain available. Nonconflicting legacy catalogs migrate automatically with originals preserved; conflicting definitions require reconciliation. Complete legacy recovery in its original worktree before migration, and do not mix old and new versions in one clone. -`rebase` and `sync` currently refuse foreign-owned stack members and writable trunks before requested ref, checkout, membership, or remote-stack changes. This includes members outside a rebase range and remote-added sync branches. Prerequisite catalog migration and discovery fetches may already have completed. Neither command auto-stashes or manages worktree creation/removal. Rebase `--continue` and `--abort` must be invoked in the recorded origin; modify recovery may be invoked elsewhere but executes there. Partial recovery failures retain state. See [Working across Git worktrees](/gh-stack/guides/workflows/#working-across-git-worktrees) for details. +`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. diff --git a/internal/config/config.go b/internal/config/config.go index e874e88..c924f88 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -67,7 +67,9 @@ type Config struct { type StackMutationContext struct { CommonDir string StateDir string - Kind string + + // NoCheckoutOnSelect preserves rewrite origins and range anchors. + NoCheckoutOnSelect bool } // New creates a new Config with terminal-aware output and color support. diff --git a/internal/git/git.go b/internal/git/git.go index 8608f08..2413a2f 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -151,8 +151,9 @@ func (d *defaultOps) runRebaseCommand(args []string, opts RebaseOpts) error { func rebaseArgs(opts RebaseOpts) []string { // The cascade owns its ref range and must never stash another worktree. + // Detached maintenance can race the next commit's rerere lock. // Use configuration overrides rather than flags unavailable in Git 2.36. - args := []string{"-c", "rebase.updateRefs=false", "-c", "rebase.autoStash=false", "rebase"} + args := []string{"-c", "rebase.updateRefs=false", "-c", "rebase.autoStash=false", "-c", "maintenance.auto=false", "rebase"} if opts.CommitterDateIsAuthorDate { // The apply backend loses this option after a conflict. The merge // backend persists it for continuation and rerere auto-continuation. diff --git a/internal/git/gitops_test.go b/internal/git/gitops_test.go index 66b42fe..0966112 100644 --- a/internal/git/gitops_test.go +++ b/internal/git/gitops_test.go @@ -1,11 +1,13 @@ package git import ( + "encoding/json" "fmt" "os" "os/exec" "path/filepath" "runtime" + "slices" "strings" "testing" @@ -1429,6 +1431,9 @@ func TestIntegration_WorktreeRerereAutoContinuesMultipleCommits(t *testing.T) { original := gitExec(t, path, "rev-parse", "HEAD") mainHead := gitExec(t, dir, "rev-parse", "HEAD") require.NoError(t, linked.EnableRerere()) + gitExec(t, dir, "config", "maintenance.auto", "true") + tracePath := filepath.Join(t.TempDir(), "rebase-trace") + t.Setenv("GIT_TRACE2_EVENT", tracePath) t.Setenv("GIT_EDITOR", "false") restore := forbidGlobalWorktreeQueries(t) defer restore() @@ -1465,6 +1470,31 @@ func TestIntegration_WorktreeRerereAutoContinuesMultipleCommits(t *testing.T) { require.NoError(t, err) assert.Contains(t, string(data), "resolved") } + trace, err := os.ReadFile(tracePath) + require.NoError(t, err) + var rebaseSessions []string + for _, line := range strings.Split(strings.TrimSpace(string(trace)), "\n") { + var event struct { + Event string `json:"event"` + SID string `json:"sid"` + Argv []string `json:"argv"` + } + require.NoError(t, json.Unmarshal([]byte(line), &event)) + if event.Event == "start" && slices.Contains(event.Argv, "rebase") { + rebaseSessions = append(rebaseSessions, event.SID) + } + if event.Event != "child_start" || len(event.Argv) < 3 || + event.Argv[1] != "maintenance" || event.Argv[2] != "run" || !slices.Contains(event.Argv, "--auto") { + continue + } + for _, sid := range rebaseSessions { + assert.False(t, event.SID == sid || strings.HasPrefix(event.SID, sid+"/"), + "automatic maintenance can race the next commit's rerere lock: %v", event.Argv) + } + } + assert.NotEmpty(t, rebaseSessions) + assert.Equal(t, "true", gitExec(t, dir, "config", "--local", "--get", "maintenance.auto"), + "the rebase must not change the repository's maintenance configuration") } func TestIntegration_WorktreeRebaseContinueStopsWhenNoProgress(t *testing.T) { diff --git a/internal/git/worktree_test.go b/internal/git/worktree_test.go index f6d476c..4200d74 100644 --- a/internal/git/worktree_test.go +++ b/internal/git/worktree_test.go @@ -209,7 +209,7 @@ func TestStateQueryWrappersDelegateErrors(t *testing.T) { func TestRebaseArgs(t *testing.T) { for _, date := range []bool{false, true} { args := rebaseArgs(RebaseOpts{CommitterDateIsAuthorDate: date}) - want := []string{"-c", "rebase.updateRefs=false", "-c", "rebase.autoStash=false", "rebase"} + want := []string{"-c", "rebase.updateRefs=false", "-c", "rebase.autoStash=false", "-c", "maintenance.auto=false", "rebase"} if date { want = append(want, "--merge", "--committer-date-is-author-date") } diff --git a/internal/stack/lock.go b/internal/stack/lock.go index b4a2914..ff06db6 100644 --- a/internal/stack/lock.go +++ b/internal/stack/lock.go @@ -55,9 +55,9 @@ func Lock(gitDir string) (*FileLock, error) { return lock, err } -// LockOperation serializes gh-stack mutations across the clone. Acquire it -// before loading mutation snapshots and before taking the catalog lock. -// Unlike the catalog lock, it may be held across Git operations. +// LockOperation serializes mutations in a repository's resolved common +// directory. Acquire it before loading mutation state or taking the catalog +// lock. Save only takes the catalog lock, so it may be used while this is held. func LockOperation(commonDir string) (*FileLock, error) { lock, _, err := acquireLock(filepath.Join(commonDir, operationLockFileName), "stack operation", true) return lock, err diff --git a/skills/gh-stack/SKILL.md b/skills/gh-stack/SKILL.md index 16c680e..99b6fbb 100644 --- a/skills/gh-stack/SKILL.md +++ b/skills/gh-stack/SKILL.md @@ -168,10 +168,9 @@ an ancestor of the branch. ## Constraints - Stacks are strictly linear: one parent, at most one child. Use separate stacks for parallel work. -- `rebase` and `sync` currently require all members and writable trunks to be unoccupied or owned - by the invoking worktree. They refuse distributed rewrites before requested changes, after - prerequisite catalog migration. They never auto-stash or create/remove worktrees. Mutations - serialize across the clone; rebase recovery must run in its recorded origin. +- `rebase` and `sync` automatically update affected clean worktrees; they never auto-stash or + create/remove worktrees. Mutations serialize across the clone, and paused operations require + recovery in their recorded owners. - 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. diff --git a/skills/gh-stack/references/commands.md b/skills/gh-stack/references/commands.md index 3825396..89ed80b 100644 --- a/skills/gh-stack/references/commands.md +++ b/skills/gh-stack/references/commands.md @@ -111,10 +111,9 @@ The routine command. Steps, in order: 8. **Prune** local branches for merged PRs, only when `--prune` is passed in a non-interactive environment. -Foreign-owned members, including merged branches, and foreign-owned trunks are currently refused -before requested mutations. Remote additions/replacements are checked before import. Prerequisite -catalog migration may already have completed. Cascade rollback does not undo prior fetches or -completed fast-forwards; partial restoration failures retain recovery state. +Affected clean worktrees are updated automatically. Dirty/busy/unavailable owners stop unsafe +updates, and pruning skips branches occupied elsewhere. Cascade rollback does not undo prior +fetches or completed fast-forwards; partial restoration failures retain recovery state. ## rebase @@ -130,10 +129,9 @@ to rebase only part of the stack. - A merged PR is detected automatically and replayed with `--onto` against the correct target, so a squash-merged parent does not produce spurious conflicts. - Starting a rebase while one is in progress exits **7**. -- All members must currently be unoccupied or checked out in the initiating worktree, even those - outside the selected range. Foreign-owned trunks require `--no-trunk`. Resolve/stage conflicts - at the reported path and invoke `--continue`/`--abort` there; recovery from another worktree is - refused. No auto-stash or worktree lifecycle management. +- Occupied branches are rebased in their clean owning worktrees; unoccupied branches use the + origin. Resolve/stage conflicts at the reported path. `--continue`/`--abort` may run from any + linked worktree and use the recorded owners. No auto-stash or worktree lifecycle management. ## view diff --git a/skills/gh-stack/references/troubleshooting.md b/skills/gh-stack/references/troubleshooting.md index 59b2cfc..f622e0c 100644 --- a/skills/gh-stack/references/troubleshooting.md +++ b/skills/gh-stack/references/troubleshooting.md @@ -147,11 +147,9 @@ reported source definitions rather than choosing the newest file. Finish legacy their original worktree first, and do not mix old and new gh-stack writers in one clone. Navigation does not take over another worktree's checkout. Use `--print-path` with an explicit -target, check the exit status, and change directory to the quoted output. Rebase/sync currently -refuse foreign-owned members or writable trunks rather than rewriting across worktrees. Keep -their target branches unoccupied or owned by the initiating worktree; rebase recovery must also -be invoked in that origin. Prerequisite catalog migration may finish before a rewrite refusal. -gh-stack does not automatically stash or create/remove worktrees. +target, check the exit status, and change directory to the quoted output. Only affected clean +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