Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/copilot-instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,6 @@ No Makefile, no code generation, no external linter config. Standard Go toolchai
- Use `stackStateDir(cfg)` for application state and `beginStackMutation` before mutation snapshots; defer cleanup. The clone-wide operation lock is separate from short catalog saves.
- Recovery must match stack identity, execute in the recorded worktree, and retain journals on partial failures. Native Git markers stay per-worktree.
- Mutation locks coordinate gh-stack only, not Git commands/editors. Keep affected worktrees quiescent during rewrites. Pass the snapshot SHA (or prior `Context.Touched` SHA) to `Context.Start` before ref mutations; never claim an external commit as this operation's work during continuation.
- Core modify rejects distributed stack branches before TUI/apply; foreign trunk ownership alone is allowed. Do not enable distributed modify until its dependent layer is implemented.
- Distributed modify preflights action/cascade targets and executes in recorded owners; only the origin may switch for unoccupied branches. Preserve dropped/folded source worktrees and refs. Before native continuation, preflight other worktrees, not remaining branches in the intentionally busy pending worktree.

For full architecture details, see [AGENTS.md](../AGENTS.md) in the repository root.
4 changes: 2 additions & 2 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,8 @@ if errors.As(err, &exitErr) { ... }
- **Migration:** Consolidate only nonconflicting legacy catalogs and preserve originals. Stop on conflicting definitions; finish legacy recovery in its original worktree before migrating. Do not mix old and new writers.
- **Recovery:** gh-stack journals live in the common directory and record origin/owner identities, original refs, and progress. Native Git markers remain per-worktree. Continue/abort must use recorded scoped executors, match stack identity (not catalog array position), and retain state on any partial restore or save failure.
- **External changes:** Mutation locks coordinate gh-stack, not arbitrary Git commands or editors. Keep affected worktrees quiescent during rewrites, except for requested conflict resolution while paused. Call `Context.Start(branch, expectedSHA)` before ref mutations: use the original snapshot SHA, or the last `Context.Touched` SHA for a branch already changed by this operation. Do not adopt a freshly read tip as this operation's baseline during continuation.
- **Separate Git directories:** Native topology may report the administration directory as the main worktree path for `--separate-git-dir` repositories. A known origin remains usable, but a foreign main-owner root may be undiscoverable. Never infer a working directory from an administration path, emit it as a successful navigation target, or add a private registry/config mutation to guess ownership.
- **Core modify boundary:** Plain modify permits unoccupied branches and branches owned by its origin worktree, but rejects distributed stack membership before the TUI/apply. Trunk ownership alone does not block it. Full distributed modify is a separate layer.
- **Separate Git directories:** Main-worktree invocation and existing absolute/relative `core.worktree` backlinks, including the main `config.worktree`, are supported. The discovery caveat is only linked invocation without a main-worktree backlink: fail actionably if that owner is required, but allow unaffected worktrees to proceed. Never infer a working directory from an administration path, emit it as a successful navigation target, or add a private registry/config mutation to guess ownership.
- **Distributed modify:** Preflight the staged actions and surviving cascade branches before mutation. Run renames and history rewrites in recorded owners, using the origin only for unoccupied branches. Persist execution progress, rename aliases, created refs, and last-written heads. Before native continuation, preflight other target worktrees; the pending worktree is intentionally busy and its later branches are checked after continuation. Never switch a foreign owner to another branch, reset external commits, delete a preserved source branch/worktree, or clear a partially restored journal.

## CI workflows (`.github/workflows/`)

Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,9 @@ Mutation locks coordinate **gh-stack processes only**, not arbitrary Git command

Navigation to a branch checked out elsewhere reports its path and fails without switching. Add `--print-path` to `up`, `down`, `top`, `bottom`, `trunk`, or an explicit-target `checkout` to get the owning path instead. Unoccupied targets are checked out here before printing this worktree's path; successful stdout contains only the absolute path and a newline. See the [worktree workflow](docs/src/content/docs/guides/workflows.md#working-across-git-worktrees) for a shell wrapper that checks errors before changing directories.

**Temporary core limitation:** `modify` works inside a linked worktree only when all stack branches are unoccupied or checked out in that same worktree. Distributed modify is rejected before opening the TUI or applying changes. A trunk checked out elsewhere is allowed because modify only reads it.
`modify` supports stacks distributed across worktrees. It preflights the owners needed by the staged actions and cascade, renames in each branch's owner, and cherry-picks or rebases in the receiving owner's worktree. Unoccupied branches use the initiating worktree; other worktrees are never switched to different branches. Dropped/folded branches and their worktrees remain intact. If the origin's nearest surviving branch is owned elsewhere, modify keeps the preserved original branch checked out and reports the survivor's path. Trunk is only read.

With `git init --separate-git-dir`, Git may report the administration directory instead of the main working directory in its worktree list. Shared storage and operations from a known main or linked worktree still work, but discovering that main worktree's owner path from another checkout can be unavailable. Do not treat an administration-directory path as a checkout directory; use the actual main worktree when its location cannot be discovered.
Repositories created with `git init --separate-git-dir` support main-worktree invocation and main-owner discovery through an existing absolute or relative `core.worktree` backlink, including one stored in the main `config.worktree`. The discovery limitation is only linked-worktree invocation without a main-worktree backlink: an operation requiring that main owner fails with actionable guidance, while unaffected worktrees continue. An administration directory is never treated as a checkout destination.

## Commands

Expand Down
8 changes: 6 additions & 2 deletions cmd/modify.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ Operations available:
• Rename branches

All changes are staged in the TUI and applied together when you press Ctrl+S.
Branches may be checked out in different worktrees. Changes run in their clean
owning worktrees; unoccupied branches use the initiating worktree. Foreign
worktrees are never switched to different branches. Changes are never autostashed,
and no worktrees are created or removed.
If your changes affect branches with pull requests, run 'gh stack submit'
afterward to push changes, update PRs, and recreate the stack on GitHub.`,
Example: ` # Open the interactive TUI to restructure the stack
Expand Down Expand Up @@ -168,11 +172,11 @@ func runModify(cfg *config.Config) error {
if state == nil || state.Worktrees == nil {
return fmt.Errorf("modify conflict has no recorded worktree; recovery state was retained")
}
ops, err := state.Worktrees.OriginOps()
ops, path, err := modify.ConflictOps(state)
if err != nil {
return err
}
printConflictDetailsAt(cfg, ops, state.Worktrees.Origin.Path, conflict.Branch, "gh stack modify --continue")
printConflictDetailsAt(cfg, ops, path, conflict.Branch, "gh stack modify --continue")
cfg.Printf("")

cfg.Printf("Or restore the stack to its pre-modify state with `%s`",
Expand Down
203 changes: 193 additions & 10 deletions cmd/modify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"testing"
"time"

tea "github.com/charmbracelet/bubbletea"
"github.com/github/gh-stack/internal/config"
"github.com/github/gh-stack/internal/git"
"github.com/github/gh-stack/internal/github"
Expand Down Expand Up @@ -924,6 +925,12 @@ func TestCheckModifyPreconditions_Worktrees(t *testing.T) {
}, nil
},
}
mock.ForWorktreeFn = func(path string) git.Ops {
if worktree.SamePath(path, foreign) {
return &git.MockOps{RootDirFn: func() (string, error) { return foreign, nil }}
}
return mock
}
restore := git.SetOps(mock)
defer restore()
cfg, _, errR := config.NewTestConfig()
Expand All @@ -940,14 +947,9 @@ func TestCheckModifyPreconditions_Worktrees(t *testing.T) {
cfg.Err.Close()
output, readErr := io.ReadAll(errR)
require.NoError(t, readErr)
if ownerBranch == "b2" {
require.Error(t, err)
assert.Contains(t, string(output), "distributed modify is not supported yet")
assert.Contains(t, string(output), foreign)
assert.Zero(t, prQueries.Load(), "distributed guard must run before PR refresh or TUI")
} else {
require.NoError(t, err)
}
require.NoError(t, err)
assert.NotContains(t, string(output), "distributed modify is not supported")
assert.Positive(t, prQueries.Load(), "action-specific owner checks happen after the TUI produces its plan")
assert.False(t, modify.StateExists(dir))
})
}
Expand Down Expand Up @@ -991,12 +993,18 @@ func TestRunModifyRecovery_UsesRecordedOrigin(t *testing.T) {
require.NoError(t, modify.SaveState(common, state))
inProgress, continued, aborted := true, false, false
sha := "original"
revParse := func(ref string) (string, error) {
if ref == "B" {
return "source", nil
}
return sha, nil
}
originOps := &git.MockOps{
GitDirFn: func() (string, error) { return originDir, nil },
CommonDirFn: func() (string, error) { return common, nil },
RootDirFn: func() (string, error) { return origin, nil },
CurrentBranchFn: func() (string, error) { return "A", nil },
RevParseFn: func(string) (string, error) { return sha, nil },
RevParseFn: revParse,
IsRebaseInProgressFn: func() bool { return inProgress && tc.conflictType == "rebase" },
RebaseContinueFn: func(git.RebaseOpts) error {
require.Equal(t, "rebase", tc.conflictType)
Expand Down Expand Up @@ -1026,7 +1034,7 @@ func TestRunModifyRecovery_UsesRecordedOrigin(t *testing.T) {
CommonDirFn: func() (string, error) { return common, nil },
RootDirFn: func() (string, error) { return caller, nil },
CurrentBranchFn: func() (string, error) { return "observer", nil },
RevParseFn: func(string) (string, error) { return sha, nil },
RevParseFn: revParse,
CheckoutBranchFn: func(string) error {
callerSensitiveCalls++
return nil
Expand Down Expand Up @@ -1099,6 +1107,49 @@ func TestModifyStateIOFailures(t *testing.T) {
require.Error(t, modify.CheckStateGuard(dir))
}

func TestModifyApply_DoesNotReportAdministrationDirectoryAsOwner(t *testing.T) {
dir, origin := t.TempDir(), t.TempDir()
s := stack.Stack{Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "A"}, {Branch: "B"}}}
writeStackFile(t, dir, s)
mock := &git.MockOps{
GitDirFn: func() (string, error) { return filepath.Join(dir, "worktrees", "origin"), nil },
CommonDirFn: func() (string, error) { return dir, nil },
RootDirFn: func() (string, error) { return origin, nil },
CurrentBranchFn: func() (string, error) { return "B", nil },
BranchExistsFn: func(string) bool { return true },
RevParseFn: func(ref string) (string, error) { return "sha-" + ref, nil },
WorktreesFn: func() ([]git.Worktree, error) {
return []git.Worktree{{Path: dir, Branch: "A"}, {Path: origin, Branch: "B"}}, nil
},
}
mock.ForWorktreeFn = func(path string) git.Ops {
if worktree.SamePath(path, dir) {
return &git.MockOps{
GitDirFn: func() (string, error) { return dir, nil },
CommonDirFn: func() (string, error) { return dir, nil },
RootDirFn: func() (string, error) { return "", assert.AnError },
}
}
return mock
}
restore := git.SetOps(mock)
defer restore()
cfg, _, _ := config.NewTestConfig()
defer cfg.Out.Close()
defer cfg.Err.Close()
sf, err := stack.Load(dir)
require.NoError(t, err)
nodes := []modifyview.ModifyBranchNode{
{BranchNode: stackview.BranchNode{Ref: s.Branches[1]}, OriginalPosition: 1},
{BranchNode: stackview.BranchNode{Ref: s.Branches[0]}, OriginalPosition: 0},
}
_, _, err = modify.ApplyPlan(cfg, dir, &sf.Stacks[0], sf, nodes, "B", func(*stack.Stack) {})
require.Error(t, err)
assert.Contains(t, err.Error(), "working-tree root")
assert.NotContains(t, err.Error(), "checked out in worktree "+dir)
assert.False(t, modify.StateExists(dir))
}

func TestRunModifyContinue_LegacyPrivateJournalKeepsOriginalCatalog(t *testing.T) {
common, origin := t.TempDir(), t.TempDir()
private := filepath.Join(common, "worktrees", "legacy")
Expand Down Expand Up @@ -1172,3 +1223,135 @@ func TestRunModifyContinue_LegacyPrivateJournalKeepsOriginalCatalog(t *testing.T
require.NoError(t, err)
assert.Equal(t, []string{"A", "B", "C"}, privateCatalog.Stacks[0].BranchNames())
}

func TestRunModifyContinue_UsesForeignPendingOwner(t *testing.T) {
common, origin, target, caller := t.TempDir(), t.TempDir(), t.TempDir(), t.TempDir()
refs := map[string]string{"main": "sha-main", "A": "sha-A", "C": "sha-C"}
s := stack.Stack{Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "A"}, {Branch: "C"}}}
writeStackFile(t, common, s)
metadata, err := json.Marshal(s)
require.NoError(t, err)
state := &modify.StateFile{
SchemaVersion: 1, Phase: modify.PhaseConflict, ConflictType: "rebase", ConflictBranch: "A",
OriginalBranch: "C", RemainingBranches: []string{"C"}, OriginalRefs: map[string]string{"C": "sha-A"},
Snapshot: modify.Snapshot{
StackMetadata: metadata,
Branches: []modify.BranchSnapshot{{Name: "A", TipSHA: "sha-A"}, {Name: "C", TipSHA: "sha-C"}},
},
Worktrees: &worktree.Context{
Origin: worktree.Location{Path: origin},
Owners: map[string]*worktree.Location{"A": {Path: target}},
Pending: "A",
PendingBefore: "sha-A",
},
}
state.RecordStack(&s)
require.NoError(t, modify.SaveState(common, state))
scoped := func(path, name string) *git.MockOps {
return &git.MockOps{
RootDirFn: func() (string, error) { return path, nil },
CommonDirFn: func() (string, error) { return common, nil },
GitDirFn: func() (string, error) { return filepath.Join(common, "worktrees", name), nil },
CurrentBranchFn: func() (string, error) { return name, nil },
RevParseFn: func(ref string) (string, error) { return refs[ref], nil },
IsAncestorFn: func(string, string) (bool, error) { return true, nil },
MergeBaseFn: func(string, string) (string, error) { return "sha-A", nil },
RebaseContinueFn: func(git.RebaseOpts) error {
t.Fatal("native continuation must run only in the pending owner's worktree")
return nil
},
}
}
originOps, targetOps, callerOps := scoped(origin, "C"), scoped(target, "A"), scoped(caller, "observer")
inProgress, continued := true, false
targetOps.IsRebaseInProgressFn = func() bool { return inProgress }
targetOps.RebaseContinueFn = func(git.RebaseOpts) error { inProgress, continued = false, true; return nil }
callerOps.WorktreesFn = func() ([]git.Worktree, error) {
return []git.Worktree{{Path: origin, Branch: "C"}, {Path: target, Branch: "A"}, {Path: caller, Branch: "observer"}}, nil
}
callerOps.ForWorktreeFn = func(path string) git.Ops {
if worktree.SamePath(path, target) {
return targetOps
}
if worktree.SamePath(path, origin) {
return originOps
}
return callerOps
}
restore := git.SetOps(callerOps)
defer restore()
nativeOps, path, err := modify.ConflictOps(state)
require.NoError(t, err)
assert.Same(t, targetOps, nativeOps)
assert.Equal(t, target, path)
cfg, _, _ := config.NewTestConfig()
defer cfg.Out.Close()
defer cfg.Err.Close()
require.NoError(t, runModifyContinue(cfg))
assert.True(t, continued)
assert.False(t, modify.StateExists(common))
assert.Nil(t, cfg.StackMutation)
}

func TestModifyTUI_RejectsMixedReorderFold(t *testing.T) {
for _, scenario := range []struct {
name string
keys []rune
order []string
kind modifyview.ActionType
}{
{"move B below A then fold up", []rune{'J', 'u'}, []string{"C", "A", "B"}, modifyview.ActionMove},
{"fold B up then move", []rune{'u', 'J'}, []string{"C", "B", "A"}, modifyview.ActionFoldUp},
} {
t.Run(scenario.name, func(t *testing.T) {
nodes := []modifyview.ModifyBranchNode{
{BranchNode: stackview.BranchNode{Ref: stack.BranchRef{Branch: "C"}}, OriginalPosition: 0},
{BranchNode: stackview.BranchNode{Ref: stack.BranchRef{Branch: "B"}, IsCurrent: true}, OriginalPosition: 1},
{BranchNode: stackview.BranchNode{Ref: stack.BranchRef{Branch: "A"}}, OriginalPosition: 2},
}
model := modifyview.New(nodes, stack.BranchRef{Branch: "main"}, "test")
for _, key := range scenario.keys {
updated, _ := model.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{key}})
var ok bool
model, ok = updated.(modifyview.Model)
require.True(t, ok)
}
var order []string
for _, node := range model.Nodes() {
order = append(order, node.Ref.Branch)
if scenario.kind == modifyview.ActionMove {
assert.Nil(t, node.PendingAction, "fold must be rejected after reordering")
assert.False(t, node.Removed)
}
}

assert.Equal(t, scenario.order, order)
require.Len(t, model.StagedActions(), 1, "only the first operation may be staged")
assert.Equal(t, scenario.kind, model.StagedActions()[0].Type)
})
}
}

func TestModifyTUI_DropThenFoldUpSkipsDroppedNeighbor(t *testing.T) {
nodes := []modifyview.ModifyBranchNode{
{BranchNode: stackview.BranchNode{Ref: stack.BranchRef{Branch: "C"}}, OriginalPosition: 0},
{BranchNode: stackview.BranchNode{Ref: stack.BranchRef{Branch: "B"}, IsCurrent: true}, OriginalPosition: 1},
{BranchNode: stackview.BranchNode{Ref: stack.BranchRef{Branch: "A"}}, OriginalPosition: 2},
}
model := modifyview.New(nodes, stack.BranchRef{Branch: "main"}, "test")
for _, key := range []rune{'x', 'j', 'u'} {
updated, _ := model.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{key}})
var ok bool
model, ok = updated.(modifyview.Model)
require.True(t, ok)
}
actions := model.StagedActions()
require.Len(t, actions, 2)
assert.Equal(t, modifyview.ActionDrop, actions[0].Type)
assert.Equal(t, "B", actions[0].BranchName)
assert.Equal(t, modifyview.ActionFoldUp, actions[1].Type)
assert.Equal(t, "A", actions[1].BranchName)
assert.Equal(t, "C", actions[1].FoldTarget)
assert.True(t, model.Nodes()[1].Removed)
assert.True(t, model.Nodes()[2].Removed)
}
2 changes: 1 addition & 1 deletion docs/src/content/docs/getting-started/quick-start.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ Linked worktrees share the same local stack catalog. You can adopt branches alre

`rebase` and `sync` automatically update affected clean owners. gh-stack does not auto-stash or manage worktree creation/removal. To navigate across worktrees, use `--print-path` and a shell wrapper that checks the command's exit status before `cd`; see [Working across Git worktrees](/gh-stack/guides/workflows/#working-across-git-worktrees).

For this core release, `modify` supports a stack within one worktree but temporarily rejects stack branches checked out in other worktrees.
`modify` can restructure a stack distributed across worktrees. It uses each affected branch's clean owner without switching other worktrees, and preserves the underlying branches/worktrees when dropping or folding layers. Conflict messages identify where to resolve and stage; `modify --continue` and `--abort` can be invoked from any linked worktree.

## What's Next?

Expand Down
Loading
Loading