Skip to content

Add worktree-scoped Git execution #526

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
skarim merged 2 commits into from
Oct 1, 2026
+2,884 −401
Merged

Add worktree-scoped Git execution #526

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
94dbc83
Add worktree-scoped Git execution
skarim Sep 28, 2026
1f5bcda
Propagate scoped Git state lookup errors
skarim Sep 29, 2026
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
1 change: 1 addition & 0 deletions AGENTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ if errors.As(err, &exitErr) { ... }
### Key interfaces

- **`git.Ops`** (`internal/git/gitops.go`): 52 methods wrapping git CLI calls. The production implementation uses `cli/go-gh`'s `client.Command()` via `run()` and `runSilent()` helpers. Package-level functions (e.g., `git.CurrentBranch()`) delegate to a swappable package-level `ops` variable.
- **Scoped Git errors:** `ForWorktree(path)` returns `(Ops, error)` and no executor for invalid contexts. Each scoped operation rechecks directory identity. `BranchExists`, `HasStagedChanges`, `IsRebaseInProgress`, and `IsCherryPickInProgress` return `(bool, error)`; callers must handle lookup failures before mutating Git or recovery state, not treat them as absence.
- **`github.ClientOps`** (`internal/github/client_interface.go`): 18 methods for GitHub API (PRs, stacks, merges). Stack operations use the public Stacks REST API (`/repos/{owner}/{repo}/stacks`): `ListStacks`, `FindStackForPR`, `GetStack`, `CreateStack`, `AddToStack` (delta append), `Unstack`. Async stack merges use `RepoMergeConfig` (GraphQL: allowed merge methods + viewer's default), `BaseBranchUsesMergeQueue` (GraphQL: detects a base-branch merge queue to select the explicit `merge_action`), `MergeStackAsync`, and `GetAsyncMergeResult` (`/repos/{owner}/{repo}/pulls/{n}/merge-async`). Injected via `cfg.GitHubClientOverride` in tests.
- **`config.Config`** (`internal/config/config.go`): Central configuration passed to all commands. Holds I/O streams, color functions, and test hook fields (`SelectFn`, `ConfirmFn`, `InputFn`, `RepoOverride`).

Expand Down
13 changes: 11 additions & 2 deletions cmd/add.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,11 @@ func runAdd(cfg *config.Config, opts *addOptions, args []string) error {

// If the branch already exists in git but is not part of any stack,
// adopt it instead of erroring. This mirrors the init command's behavior.
adopted := git.BranchExists(branchName)
adopted, err := git.BranchExists(branchName)
if err != nil {
cfg.Errorf("failed to check branch %s: %s", branchName, err)
return ErrSilent
}
var adoptedBase string
if adopted {
adoptedBase, err = git.MergeBase(currentBranch, branchName)
Expand Down Expand Up @@ -331,7 +335,12 @@ func stageAndValidate(cfg *config.Config, opts *addOptions) error {
}
}

if !git.HasStagedChanges() {
staged, err := git.HasStagedChanges()
if err != nil {
cfg.Errorf("failed to check staged changes: %s", err)
return err
}
if !staged {
if opts.stageAll || opts.stageTracked {
cfg.Errorf("no changes to commit after staging")
} else {
Expand Down
75 changes: 62 additions & 13 deletions cmd/add_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,55 @@ func saveStack(t *testing.T, gitDir string, s stack.Stack) {
require.NoError(t, stack.Save(gitDir, sf), "saving seed stack")
}

func TestAdd_StateLookupFailureDoesNotMutate(t *testing.T) {
for _, query := range []string{"branch", "staged"} {
t.Run(query, func(t *testing.T) {
gitDir := t.TempDir()
s := stack.Stack{
Trunk: stack.BranchRef{Branch: "main"},
Branches: []stack.BranchRef{{Branch: "b1"}},
}
saveStack(t, gitDir, s)
lookupErr := fmt.Errorf("state lookup failed")
mock := &git.MockOps{
GitDirFn: func() (string, error) { return gitDir, nil },
CurrentBranchFn: func() (string, error) { return "b1", nil },
RevParseMultiFn: func([]string) ([]string, error) {
return []string{"parent", "head"}, nil
},
CreateBranchFn: func(string, string) error {
t.Fatal("must not create a branch after a failed lookup")
return nil
},
CheckoutBranchFn: func(string) error {
t.Fatal("must not check out a branch after a failed lookup")
return nil
},
CommitFn: func(string) (string, error) {
t.Fatal("must not commit after a failed lookup")
return "", nil
},
}
if query == "branch" {
mock.BranchExistsFn = func(string) (bool, error) { return true, lookupErr }
} else {
mock.HasStagedChangesFn = func() (bool, error) { return true, lookupErr }
}
restore := git.SetOps(mock)
defer restore()
cfg, outR, errR := config.NewTestConfig()
err := runAdd(cfg, &addOptions{message: "new commit"}, []string{"new-branch"})
require.ErrorIs(t, err, ErrSilent)
output := collectOutput(cfg, outR, errR)
assert.Contains(t, output, lookupErr.Error())
assert.NotContains(t, output, "nothing to commit")
sf, err := stack.Load(gitDir)
require.NoError(t, err)
assert.Equal(t, s, sf.Stacks[0])
})
}
}

func TestAdd_CreatesNewBranch(t *testing.T) {
gitDir := t.TempDir()
saveStack(t, gitDir, stack.Stack{
Expand Down Expand Up @@ -113,7 +162,7 @@ func TestAdd_StagingWithoutMessageUsesEditor(t *testing.T) {
CreateBranchFn: func(name, base string) error { return nil },
CheckoutBranchFn: func(name string) error { return nil },
StageAllFn: func() error { return nil },
HasStagedChangesFn: func() bool { return true },
HasStagedChangesFn: func() (bool, error) { return true, nil },
CommitInteractiveFn: func() (string, error) {
interactiveCalled = true
return "def1234567890", nil
Expand Down Expand Up @@ -149,7 +198,7 @@ func TestAdd_EmptyBranchCommitsInPlace(t *testing.T) {
stageAllCalled = true
return nil
},
HasStagedChangesFn: func() bool { return true },
HasStagedChangesFn: func() (bool, error) { return true, nil },
CommitFn: func(msg string) (string, error) {
commitCalled = true
return "abc1234567890", nil
Expand Down Expand Up @@ -197,7 +246,7 @@ func TestAdd_BranchWithCommitsCreatesNew(t *testing.T) {
checkoutCalled = true
return nil
},
HasStagedChangesFn: func() bool { return true },
HasStagedChangesFn: func() (bool, error) { return true, nil },
CommitFn: func(msg string) (string, error) {
commitCalled = true
return "def1234567890", nil
Expand Down Expand Up @@ -259,7 +308,7 @@ func TestAdd_MessageAutoGeneratesDateSlug(t *testing.T) {
createdBranch = name
return nil
},
HasStagedChangesFn: func() bool { return true },
HasStagedChangesFn: func() (bool, error) { return true, nil },
CommitFn: func(msg string) (string, error) {
return "def1234567890", nil
},
Expand Down Expand Up @@ -312,7 +361,7 @@ func TestAdd_NothingToCommit(t *testing.T) {
return []string{"aaa", "aaa"}, nil // same SHA = empty branch
},
StageAllFn: func() error { return nil },
HasStagedChangesFn: func() bool { return false },
HasStagedChangesFn: func() (bool, error) { return false, nil },
})
defer restore()

Expand Down Expand Up @@ -447,7 +496,7 @@ func TestAdd_AdoptsExistingBranch(t *testing.T) {
restore := git.SetOps(&git.MockOps{
GitDirFn: func() (string, error) { return gitDir, nil },
CurrentBranchFn: func() (string, error) { return "b1", nil },
BranchExistsFn: func(name string) bool { return name == "existing-branch" },
BranchExistsFn: func(name string) (bool, error) { return name == "existing-branch", nil },
MergeBaseFn: func(parent, branch string) (string, error) {
assert.Equal(t, "b1", parent)
assert.Equal(t, "existing-branch", branch)
Expand Down Expand Up @@ -504,7 +553,7 @@ func TestAdd_RejectsExistingBranchInStack(t *testing.T) {
restore := git.SetOps(&git.MockOps{
GitDirFn: func() (string, error) { return gitDir, nil },
CurrentBranchFn: func() (string, error) { return "b1", nil },
BranchExistsFn: func(name string) bool { return name == "taken-branch" },
BranchExistsFn: func(name string) (bool, error) { return name == "taken-branch", nil },
})
defer restore()

Expand All @@ -528,7 +577,7 @@ func TestAdd_AdoptsExistingBranchWithCommit(t *testing.T) {
restore := git.SetOps(&git.MockOps{
GitDirFn: func() (string, error) { return gitDir, nil },
CurrentBranchFn: func() (string, error) { return "b1", nil },
BranchExistsFn: func(name string) bool { return name == "existing-branch" },
BranchExistsFn: func(name string) (bool, error) { return name == "existing-branch", nil },
MergeBaseFn: func(string, string) (string, error) { return "common-base", nil },
RevParseMultiFn: func(refs []string) ([]string, error) {
return []string{"aaa", "bbb"}, nil // different SHAs = branch has commits
Expand All @@ -540,7 +589,7 @@ func TestAdd_AdoptsExistingBranchWithCommit(t *testing.T) {
CheckoutBranchFn: func(name string) error { return nil },
RevParseFn: func(ref string) (string, error) { return "abc", nil },
StageAllFn: func() error { return nil },
HasStagedChangesFn: func() bool { return true },
HasStagedChangesFn: func() (bool, error) { return true, nil },
CommitFn: func(msg string) (string, error) {
commitCalled = true
return "def1234567890", nil
Expand Down Expand Up @@ -570,7 +619,7 @@ func TestAdd_AdoptExistingBranchWithoutCommonBaseFails(t *testing.T) {
restore := git.SetOps(&git.MockOps{
GitDirFn: func() (string, error) { return gitDir, nil },
CurrentBranchFn: func() (string, error) { return "b1", nil },
BranchExistsFn: func(name string) bool { return name == "unrelated" },
BranchExistsFn: func(name string) (bool, error) { return name == "unrelated", nil },
MergeBaseFn: func(string, string) (string, error) { return "", assert.AnError },
CheckoutBranchFn: func(string) error {
checkedOut = true
Expand Down Expand Up @@ -602,7 +651,7 @@ func TestAdd_InitializesStackWithExplicitBranch(t *testing.T) {
CurrentBranchFn: func() (string, error) { return "unstacked", nil },
DefaultBranchFn: func() (string, error) { return "main", nil },
IsRerereEnabledFn: func() (bool, error) { return true, nil },
BranchExistsFn: func(name string) bool { return name == "main" && trunkExists },
BranchExistsFn: func(name string) (bool, error) { return name == "main" && trunkExists, nil },
RevParseFn: func(ref string) (string, error) {
if ref == "main" && !trunkExists {
return "", fmt.Errorf("unknown revision %s", ref)
Expand Down Expand Up @@ -697,7 +746,7 @@ func TestAdd_InitializesGeneratedBranchAndCommits(t *testing.T) {
CurrentBranchFn: func() (string, error) { return currentBranch, nil },
DefaultBranchFn: func() (string, error) { return "main", nil },
IsRerereEnabledFn: func() (bool, error) { return true, nil },
BranchExistsFn: func(name string) bool { return name == "main" },
BranchExistsFn: func(name string) (bool, error) { return name == "main", nil },
CreateBranchFn: func(name, base string) error {
assert.Equal(t, expectedBranch, name)
assert.Equal(t, "refs/heads/main", base)
Expand All @@ -712,7 +761,7 @@ func TestAdd_InitializesGeneratedBranchAndCommits(t *testing.T) {
stageAllCalled = true
return nil
},
HasStagedChangesFn: func() bool { return true },
HasStagedChangesFn: func() (bool, error) { return true, nil },
CommitFn: func(message string) (string, error) {
assert.Equal(t, "First layer", message)
assert.Equal(t, expectedBranch, currentBranch)
Expand Down
4 changes: 2 additions & 2 deletions cmd/checkout_picker_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ func TestCheckout_NoTarget_ConfirmedRemoteMatch(t *testing.T) {
restore := git.SetOps(&git.MockOps{
GitDirFn: func() (string, error) { return gitDir, nil },
CurrentBranchFn: func() (string, error) { return currentBranch, nil },
BranchExistsFn: func(name string) bool { return branches[name] },
BranchExistsFn: func(name string) (bool, error) { return branches[name], nil },
FetchFn: func(string) error { return nil },
ResolveRemoteFn: func(string) (string, error) { return "origin", nil },
CreateBranchFn: func(name, _ string) error {
Expand Down Expand Up @@ -393,7 +393,7 @@ func TestResolveCheckoutSelection_RemoteRoutesToClone(t *testing.T) {
restore := git.SetOps(&git.MockOps{
GitDirFn: func() (string, error) { return gitDir, nil },
CurrentBranchFn: func() (string, error) { return "main", nil },
BranchExistsFn: func(name string) bool { return name == "main" },
BranchExistsFn: func(name string) (bool, error) { return name == "main", nil },
FetchFn: func(remote string) error { return nil },
CreateBranchFn: func(name, base string) error {
createdBranches = append(createdBranches, name)
Expand Down
28 changes: 14 additions & 14 deletions cmd/checkout_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func TestCheckout_ByRemoteBranchName(t *testing.T) {
restore := git.SetOps(&git.MockOps{
GitDirFn: func() (string, error) { return gitDir, nil },
CurrentBranchFn: func() (string, error) { return "main", nil },
BranchExistsFn: func(name string) bool { return name == "main" },
BranchExistsFn: func(name string) (bool, error) { return name == "main", nil },
FetchFn: func(string) error { return nil },
CreateBranchFn: func(name, _ string) error {
createdBranches = append(createdBranches, name)
Expand Down Expand Up @@ -407,8 +407,8 @@ func TestCheckout_NumericTarget_NewStack(t *testing.T) {
restore := git.SetOps(&git.MockOps{
GitDirFn: func() (string, error) { return gitDir, nil },
CurrentBranchFn: func() (string, error) { return "main", nil },
BranchExistsFn: func(name string) bool {
return name == "main" // only trunk exists
BranchExistsFn: func(name string) (bool, error) {
return name == "main", nil // only trunk exists
},
FetchFn: func(remote string) error { return nil },
CreateBranchFn: func(name, base string) error {
Expand Down Expand Up @@ -494,7 +494,7 @@ func TestCheckout_ByStackNumber(t *testing.T) {
restore := git.SetOps(&git.MockOps{
GitDirFn: func() (string, error) { return gitDir, nil },
CurrentBranchFn: func() (string, error) { return "main", nil },
BranchExistsFn: func(name string) bool { return name == "main" },
BranchExistsFn: func(name string) (bool, error) { return name == "main", nil },
FetchFn: func(remote string) error { return nil },
CreateBranchFn: func(name, base string) error {
createdBranches = append(createdBranches, name)
Expand Down Expand Up @@ -563,7 +563,7 @@ func TestCheckout_ByStackNumber_404FallsThroughToPR(t *testing.T) {
restore := git.SetOps(&git.MockOps{
GitDirFn: func() (string, error) { return gitDir, nil },
CurrentBranchFn: func() (string, error) { return "main", nil },
BranchExistsFn: func(name string) bool { return name == "main" },
BranchExistsFn: func(name string) (bool, error) { return name == "main", nil },
FetchFn: func(string) error { return nil },
CreateBranchFn: func(string, string) error { return nil },
SetUpstreamTrackingFn: func(string, string) error { return nil },
Expand Down Expand Up @@ -611,9 +611,9 @@ func TestCheckout_NumericTarget_BranchExistsNoStack(t *testing.T) {
restore := git.SetOps(&git.MockOps{
GitDirFn: func() (string, error) { return gitDir, nil },
CurrentBranchFn: func() (string, error) { return "main", nil },
BranchExistsFn: func(name string) bool {
BranchExistsFn: func(name string) (bool, error) {
// feat-1 exists locally but feat-2 does not
return name == "main" || name == "feat-1"
return name == "main" || name == "feat-1", nil
},
FetchFn: func(remote string) error { return nil },
CreateBranchFn: func(name, base string) error {
Expand Down Expand Up @@ -723,8 +723,8 @@ func TestCheckout_NumericTarget_LocalMiss_RemoteMatch(t *testing.T) {
restore := git.SetOps(&git.MockOps{
GitDirFn: func() (string, error) { return gitDir, nil },
CurrentBranchFn: func() (string, error) { return "main", nil },
BranchExistsFn: func(name string) bool {
return name == "main"
BranchExistsFn: func(name string) (bool, error) {
return name == "main", nil
},
FetchFn: func(remote string) error { return nil },
CreateBranchFn: func(name, base string) error { return nil },
Expand Down Expand Up @@ -870,8 +870,8 @@ func TestCheckout_NumericTarget_ClosedMergedPR(t *testing.T) {
restore := git.SetOps(&git.MockOps{
GitDirFn: func() (string, error) { return gitDir, nil },
CurrentBranchFn: func() (string, error) { return "main", nil },
BranchExistsFn: func(name string) bool {
return name == "main"
BranchExistsFn: func(name string) (bool, error) {
return name == "main", nil
},
FetchFn: func(remote string) error { return nil },
CreateBranchFn: func(name, base string) error { return nil },
Expand Down Expand Up @@ -934,8 +934,8 @@ func TestCheckout_NumericTarget_MergedBranchDeletedFromRemote(t *testing.T) {
restore := git.SetOps(&git.MockOps{
GitDirFn: func() (string, error) { return gitDir, nil },
CurrentBranchFn: func() (string, error) { return "main", nil },
BranchExistsFn: func(name string) bool {
return name == "main"
BranchExistsFn: func(name string) (bool, error) {
return name == "main", nil
},
FetchFn: func(remote string) error { return nil },
CreateBranchFn: func(name, base string) error {
Expand Down Expand Up @@ -1251,7 +1251,7 @@ func TestCheckout_ByPRURL_Remote(t *testing.T) {
restore := git.SetOps(&git.MockOps{
GitDirFn: func() (string, error) { return gitDir, nil },
CurrentBranchFn: func() (string, error) { return "main", nil },
BranchExistsFn: func(name string) bool { return name == "main" },
BranchExistsFn: func(name string) (bool, error) { return name == "main", nil },
FetchFn: func(string) error { return nil },
CreateBranchFn: func(string, string) error { return nil },
SetUpstreamTrackingFn: func(string, string) error { return nil },
Expand Down
20 changes: 17 additions & 3 deletions cmd/init.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,12 @@ func runInit(cfg *config.Config, opts *initOptions) error {

// The repository's default branch may only exist on the remote if the
// initial local branch was renamed before starting the stack.
if currentBranch != trunk && !git.BranchExists(trunk) {
trunkExists, err := git.BranchExists(trunk)
if err != nil {
cfg.Errorf("failed to check trunk branch %s: %s", trunk, err)
return ErrSilent
}
if currentBranch != trunk && !trunkExists {
remote, err := pickRemote(cfg, currentBranch, "")
if err != nil {
if !errors.Is(err, errInterrupt) {
Expand Down Expand Up @@ -245,7 +250,11 @@ func resolveArgBranches(cfg *config.Config, opts *initOptions, sf *stack.StackFi
return nil, nil, ErrInvalidArgs
}

exists := git.BranchExists(b)
exists, err := git.BranchExists(b)
if err != nil {
cfg.Errorf("failed to check branch %s: %s", b, err)
return nil, nil, ErrSilent
}

if err := sf.ValidateNoDuplicateBranch(b); err != nil {
cfg.Errorf("branch %q already exists in a stack", b)
Expand Down Expand Up @@ -345,7 +354,12 @@ func runInteractiveInit(cfg *config.Config, sf *stack.StackFile, trunk, trunkRef
cfg.Errorf("branch %q already exists in a stack", branchName)
return nil, false, ErrInvalidArgs
}
if git.BranchExists(branchName) {
exists, err := git.BranchExists(branchName)
if err != nil {
cfg.Errorf("failed to check branch %s: %s", branchName, err)
return nil, false, ErrSilent
}
if exists {
wasAdopted = true
} else {
if err := git.CreateBranch(branchName, trunkRef); err != nil {
Expand Down
Loading
Loading