diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index e5adc47..66a2162 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -21,7 +21,7 @@ No Makefile, no code generation, no external linter config. Standard Go toolchai - `cmd/`: One Cobra command per file. Each exports `Cmd(cfg *config.Config)` with logic in `run()`. - `internal/git/`: `Ops` interface wrapping git CLI. `MockOps` for tests. Use `ForWorktree(path)` for scoped execution, `CommonDir()` for shared storage, and `GitDir()` for native per-worktree state. Never use production `os.Chdir` or `SetOps` to switch context. -- `internal/github/`: `ClientOps` interface (18 methods) for GitHub API. `MockClient` for tests. Stack operations use the public Stacks REST API (`/repos/{owner}/{repo}/stacks`); merges use the async merge API (`/repos/{owner}/{repo}/pulls/{n}/merge-async`) with an explicit `merge_action` (`direct_merge` or `merge_queue`) chosen from the base branch's merge-queue detection. `merge_action` is optional — omitting it (or sending `default`) lets the server auto-route (merge queue if one is configured, else direct merge) — but the CLI sends it explicitly so a wrong detection fails loudly instead of silently merging directly. +- `internal/github/`: `ClientOps` interface (19 methods) for GitHub API. `MockClient` for tests. Stack operations use the public Stacks REST API (`/repos/{owner}/{repo}/stacks`); merges use the async merge API (`/repos/{owner}/{repo}/pulls/{n}/merge-async`) with an explicit `merge_action` (`direct_merge` or `merge_queue`) chosen from the base branch's merge-queue detection. `merge_action` is optional — omitting it (or sending `default`) lets the server auto-route (merge queue if one is configured, else direct merge) — but the CLI sends it explicitly so a wrong detection fails loudly instead of silently merging directly. - `internal/config/`: `Config` struct passed to all commands. Holds I/O, colors, and test hooks (`SelectFn`, `ConfirmFn`, `InputFn`, `GitHubClientOverride`). - `internal/stack/`: Shared catalog (`/gh-stack`, JSON), conservative legacy migration, atomic saves, and short catalog locks. - `internal/worktree/`: Origin/owner identities, cleanliness preflight, scoped operations, and touched-ref recovery. diff --git a/AGENTS.md b/AGENTS.md index 1edf0dd..12e7c38 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -37,7 +37,7 @@ internal/ gitops.go # Ops interface, including scoped worktree execution mock_ops.go # MockOps. Each method has a corresponding *Fn field. github/ # github.ClientOps interface + real Client - client_interface.go # ClientOps interface (18 methods) + client_interface.go # ClientOps interface (19 methods) mock_client.go # MockClient. Uses function-pointer fields for testing. stack/ # common-directory catalog, JSON schema, migration, locking schema.json # JSON Schema for the stack file format @@ -114,7 +114,7 @@ if errors.As(err, &exitErr) { ... } - **`git.Ops`** (`internal/git/gitops.go`): wraps git CLI calls. Package-level functions (e.g., `git.CurrentBranch()`) delegate to a swappable package-level `ops` variable. `git.ForWorktree(path)` returns an explicitly scoped executor for HEAD/index/working-file operations. Never switch production context with `os.Chdir` or `git.SetOps`; reserve `SetOps` for tests. `GitDir()` remains per-worktree; `CommonDir()` is repository-wide. - **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. +- **`github.ClientOps`** (`internal/github/client_interface.go`): 19 methods for GitHub API (PRs, stacks, merges, repository default branch). 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`). ### Stack file diff --git a/cmd/add_test.go b/cmd/add_test.go index 2c9b545..e192009 100644 --- a/cmd/add_test.go +++ b/cmd/add_test.go @@ -761,7 +761,7 @@ func TestAdd_InitializesStackWithExplicitBranch(t *testing.T) { restore := git.SetOps(&git.MockOps{ GitDirFn: func() (string, error) { return gitDir, nil }, CurrentBranchFn: func() (string, error) { return "unstacked", nil }, - DefaultBranchFn: func() (string, error) { return "main", nil }, + DefaultBranchFn: func(string) (string, error) { return "main", nil }, IsRerereEnabledFn: func() (bool, error) { return true, nil }, BranchExistsFn: func(name string) (bool, error) { return name == "main" && trunkExists, nil }, RevParseFn: func(ref string) (string, error) { @@ -815,7 +815,7 @@ func TestAdd_InitializesStackWithPromptedBranch(t *testing.T) { restore := git.SetOps(&git.MockOps{ GitDirFn: func() (string, error) { return gitDir, nil }, CurrentBranchFn: func() (string, error) { return "main", nil }, - DefaultBranchFn: func() (string, error) { return "main", nil }, + DefaultBranchFn: func(string) (string, error) { return "main", nil }, IsRerereEnabledFn: func() (bool, error) { return true, nil }, CreateBranchFn: func(name, base string) error { createdBranch = name @@ -856,7 +856,7 @@ func TestAdd_InitializesGeneratedBranchAndCommits(t *testing.T) { restore := git.SetOps(&git.MockOps{ GitDirFn: func() (string, error) { return gitDir, nil }, CurrentBranchFn: func() (string, error) { return currentBranch, nil }, - DefaultBranchFn: func() (string, error) { return "main", nil }, + DefaultBranchFn: func(string) (string, error) { return "main", nil }, IsRerereEnabledFn: func() (bool, error) { return true, nil }, BranchExistsFn: func(name string) (bool, error) { return name == "main", nil }, CreateBranchFn: func(name, base string) error { diff --git a/cmd/init.go b/cmd/init.go index fe895ee..a92143b 100644 --- a/cmd/init.go +++ b/cmd/init.go @@ -87,7 +87,7 @@ func runInit(cfg *config.Config, opts *initOptions) error { } if trunk == "" { - trunk, err = git.DefaultBranch() + trunk, err = resolveDefaultBranch(cfg, "") if err != nil { cfg.Errorf("unable to determine default branch\nUse -b to specify the trunk branch") return ErrNotInStack diff --git a/cmd/init_test.go b/cmd/init_test.go index fe2785f..3ae2c91 100644 --- a/cmd/init_test.go +++ b/cmd/init_test.go @@ -102,7 +102,7 @@ func TestInit_CreatesStackWithCorrectTrunk(t *testing.T) { gitDir := t.TempDir() restore := git.SetOps(&git.MockOps{ GitDirFn: func() (string, error) { return gitDir, nil }, - DefaultBranchFn: func() (string, error) { return "main", nil }, + DefaultBranchFn: func(string) (string, error) { return "main", nil }, CurrentBranchFn: func() (string, error) { return "main", nil }, }) defer restore() @@ -151,7 +151,7 @@ func TestInit_RestoresMissingLocalTrunkWhenTagResolves(t *testing.T) { restore := git.SetOps(&git.MockOps{ GitDirFn: func() (string, error) { return gitDir, nil }, - DefaultBranchFn: func() (string, error) { return "main", nil }, + DefaultBranchFn: func(string) (string, error) { return "main", nil }, CurrentBranchFn: func() (string, error) { return "renamed-branch", nil }, IsRerereEnabledFn: func() (bool, error) { return true, nil }, BranchExistsFn: func(name string) (bool, error) { @@ -205,7 +205,7 @@ func TestInit_AdoptExistingBranches(t *testing.T) { gitDir := t.TempDir() restore := git.SetOps(&git.MockOps{ GitDirFn: func() (string, error) { return gitDir, nil }, - DefaultBranchFn: func() (string, error) { return "main", nil }, + DefaultBranchFn: func(string) (string, error) { return "main", nil }, CurrentBranchFn: func() (string, error) { return "main", nil }, BranchExistsFn: func(string) (bool, error) { return true, nil }, }) @@ -231,7 +231,7 @@ func TestInit_ExplicitBranchNamesUsedVerbatim(t *testing.T) { var created []string restore := git.SetOps(&git.MockOps{ GitDirFn: func() (string, error) { return gitDir, nil }, - DefaultBranchFn: func() (string, error) { return "main", nil }, + DefaultBranchFn: func(string) (string, error) { return "main", nil }, CurrentBranchFn: func() (string, error) { return "main", nil }, CreateBranchFn: func(name, base string) error { created = append(created, name) @@ -257,7 +257,7 @@ func TestInit_AdoptFlagShowsDeprecationWarning(t *testing.T) { gitDir := t.TempDir() restore := git.SetOps(&git.MockOps{ GitDirFn: func() (string, error) { return gitDir, nil }, - DefaultBranchFn: func() (string, error) { return "main", nil }, + DefaultBranchFn: func(string) (string, error) { return "main", nil }, CurrentBranchFn: func() (string, error) { return "main", nil }, BranchExistsFn: func(string) (bool, error) { return true, nil }, }) @@ -276,7 +276,7 @@ func TestInit_RerereAlreadyEnabled(t *testing.T) { enableRerereCalled := false restore := git.SetOps(&git.MockOps{ GitDirFn: func() (string, error) { return gitDir, nil }, - DefaultBranchFn: func() (string, error) { return "main", nil }, + DefaultBranchFn: func(string) (string, error) { return "main", nil }, CurrentBranchFn: func() (string, error) { return "main", nil }, IsRerereEnabledFn: func() (bool, error) { return true, nil }, EnableRerereFn: func() error { @@ -308,7 +308,7 @@ func TestInit_RefuseIfBranchAlreadyInStack(t *testing.T) { restore := git.SetOps(&git.MockOps{ GitDirFn: func() (string, error) { return gitDir, nil }, - DefaultBranchFn: func() (string, error) { return "main", nil }, + DefaultBranchFn: func(string) (string, error) { return "main", nil }, CurrentBranchFn: func() (string, error) { return "feature-1", nil }, }) defer restore() @@ -326,7 +326,7 @@ func TestInit_AdoptNonexistentBranch_CreatesIt(t *testing.T) { var created []string restore := git.SetOps(&git.MockOps{ GitDirFn: func() (string, error) { return gitDir, nil }, - DefaultBranchFn: func() (string, error) { return "main", nil }, + DefaultBranchFn: func(string) (string, error) { return "main", nil }, CurrentBranchFn: func() (string, error) { return "main", nil }, BranchExistsFn: func(string) (bool, error) { return false, nil }, CreateBranchFn: func(name, base string) error { @@ -350,7 +350,7 @@ func TestInit_MultipleBranches_CreatesAll(t *testing.T) { var created []string restore := git.SetOps(&git.MockOps{ GitDirFn: func() (string, error) { return gitDir, nil }, - DefaultBranchFn: func() (string, error) { return "main", nil }, + DefaultBranchFn: func(string) (string, error) { return "main", nil }, CurrentBranchFn: func() (string, error) { return "main", nil }, CreateBranchFn: func(name, base string) error { created = append(created, name) @@ -375,7 +375,7 @@ func TestInit_AdoptWithExistingOpenPR(t *testing.T) { gitDir := t.TempDir() restore := git.SetOps(&git.MockOps{ GitDirFn: func() (string, error) { return gitDir, nil }, - DefaultBranchFn: func() (string, error) { return "main", nil }, + DefaultBranchFn: func(string) (string, error) { return "main", nil }, CurrentBranchFn: func() (string, error) { return "main", nil }, BranchExistsFn: func(string) (bool, error) { return true, nil }, }) @@ -425,7 +425,7 @@ func TestInit_AdoptIgnoresClosedAndMergedPRs(t *testing.T) { gitDir := t.TempDir() restore := git.SetOps(&git.MockOps{ GitDirFn: func() (string, error) { return gitDir, nil }, - DefaultBranchFn: func() (string, error) { return "main", nil }, + DefaultBranchFn: func(string) (string, error) { return "main", nil }, CurrentBranchFn: func() (string, error) { return "main", nil }, BranchExistsFn: func(string) (bool, error) { return true, nil }, }) @@ -466,7 +466,7 @@ func TestInit_ImplicitAdopt_AllExist(t *testing.T) { gitDir := t.TempDir() restore := git.SetOps(&git.MockOps{ GitDirFn: func() (string, error) { return gitDir, nil }, - DefaultBranchFn: func() (string, error) { return "main", nil }, + DefaultBranchFn: func(string) (string, error) { return "main", nil }, CurrentBranchFn: func() (string, error) { return "main", nil }, BranchExistsFn: func(string) (bool, error) { return true, nil }, }) @@ -501,7 +501,7 @@ func TestInit_ImplicitAdopt_AllMissing(t *testing.T) { var created []string restore := git.SetOps(&git.MockOps{ GitDirFn: func() (string, error) { return gitDir, nil }, - DefaultBranchFn: func() (string, error) { return "main", nil }, + DefaultBranchFn: func(string) (string, error) { return "main", nil }, CurrentBranchFn: func() (string, error) { return "main", nil }, CreateBranchFn: func(name, base string) error { created = append(created, name) @@ -528,7 +528,7 @@ func TestInit_ImplicitAdopt_Mixed(t *testing.T) { var created []string restore := git.SetOps(&git.MockOps{ GitDirFn: func() (string, error) { return gitDir, nil }, - DefaultBranchFn: func() (string, error) { return "main", nil }, + DefaultBranchFn: func(string) (string, error) { return "main", nil }, CurrentBranchFn: func() (string, error) { return "main", nil }, BranchExistsFn: func(name string) (bool, error) { return existing[name], nil }, CreateBranchFn: func(name, base string) error { @@ -556,7 +556,7 @@ func TestInit_WhatsNext_Fresh(t *testing.T) { gitDir := t.TempDir() restore := git.SetOps(&git.MockOps{ GitDirFn: func() (string, error) { return gitDir, nil }, - DefaultBranchFn: func() (string, error) { return "main", nil }, + DefaultBranchFn: func(string) (string, error) { return "main", nil }, CurrentBranchFn: func() (string, error) { return "main", nil }, CreateBranchFn: func(name, base string) error { return nil }, }) @@ -581,7 +581,7 @@ func TestInit_WhatsNext_AdoptedWithPRs(t *testing.T) { gitDir := t.TempDir() restore := git.SetOps(&git.MockOps{ GitDirFn: func() (string, error) { return gitDir, nil }, - DefaultBranchFn: func() (string, error) { return "main", nil }, + DefaultBranchFn: func(string) (string, error) { return "main", nil }, CurrentBranchFn: func() (string, error) { return "main", nil }, BranchExistsFn: func(string) (bool, error) { return true, nil }, }) @@ -614,7 +614,7 @@ func TestInit_WhatsNext_AdoptedNoPRs(t *testing.T) { gitDir := t.TempDir() restore := git.SetOps(&git.MockOps{ GitDirFn: func() (string, error) { return gitDir, nil }, - DefaultBranchFn: func() (string, error) { return "main", nil }, + DefaultBranchFn: func(string) (string, error) { return "main", nil }, CurrentBranchFn: func() (string, error) { return "main", nil }, BranchExistsFn: func(string) (bool, error) { return true, nil }, }) @@ -639,7 +639,7 @@ func TestInit_WhatsNext_MixedWithPR(t *testing.T) { gitDir := t.TempDir() restore := git.SetOps(&git.MockOps{ GitDirFn: func() (string, error) { return gitDir, nil }, - DefaultBranchFn: func() (string, error) { return "main", nil }, + DefaultBranchFn: func(string) (string, error) { return "main", nil }, CurrentBranchFn: func() (string, error) { return "main", nil }, BranchExistsFn: func(name string) (bool, error) { return name == "existing", nil }, CreateBranchFn: func(name, base string) error { return nil }, @@ -670,7 +670,7 @@ func TestInit_Interactive_OnTrunk(t *testing.T) { gitDir := t.TempDir() restore := git.SetOps(&git.MockOps{ GitDirFn: func() (string, error) { return gitDir, nil }, - DefaultBranchFn: func() (string, error) { return "main", nil }, + DefaultBranchFn: func(string) (string, error) { return "main", nil }, CurrentBranchFn: func() (string, error) { return "main", nil }, }) defer restore() @@ -689,7 +689,7 @@ func TestInit_Interactive_OnFeatureBranch_UseCurrent(t *testing.T) { gitDir := t.TempDir() restore := git.SetOps(&git.MockOps{ GitDirFn: func() (string, error) { return gitDir, nil }, - DefaultBranchFn: func() (string, error) { return "main", nil }, + DefaultBranchFn: func(string) (string, error) { return "main", nil }, CurrentBranchFn: func() (string, error) { return "feat/auth", nil }, BranchExistsFn: func(name string) (bool, error) { return name == "feat/auth", nil }, }) @@ -732,7 +732,7 @@ func TestInit_TwoPassValidation_NoBranchCreatedOnError(t *testing.T) { var created []string restore := git.SetOps(&git.MockOps{ GitDirFn: func() (string, error) { return gitDir, nil }, - DefaultBranchFn: func() (string, error) { return "main", nil }, + DefaultBranchFn: func(string) (string, error) { return "main", nil }, CurrentBranchFn: func() (string, error) { return "main", nil }, CreateBranchFn: func(name, base string) error { created = append(created, name) @@ -756,7 +756,7 @@ func TestInit_TwoPassValidation_InvalidRefName(t *testing.T) { var created []string restore := git.SetOps(&git.MockOps{ GitDirFn: func() (string, error) { return gitDir, nil }, - DefaultBranchFn: func() (string, error) { return "main", nil }, + DefaultBranchFn: func(string) (string, error) { return "main", nil }, CurrentBranchFn: func() (string, error) { return "main", nil }, ValidateRefNameFn: func(name string) error { if name == "invalid..name" { diff --git a/cmd/link.go b/cmd/link.go index b9997cd..b8ef9e1 100644 --- a/cmd/link.go +++ b/cmd/link.go @@ -235,7 +235,7 @@ func runLinkCreateOrUpdate(cfg *config.Config, client github.ClientOps, opts *li // given, default to the repository's default branch (like `gh stack init`). bottomBase := opts.base if bottomBase == "" { - base, err := git.DefaultBranch() + base, err := resolveDefaultBranch(cfg, opts.remote) if err != nil { cfg.Errorf("unable to determine default branch\nUse --base to specify the base branch") return ErrSilent diff --git a/cmd/link_test.go b/cmd/link_test.go index d652846..20c854d 100644 --- a/cmd/link_test.go +++ b/cmd/link_test.go @@ -1409,7 +1409,7 @@ func TestLink_DefaultBase_RetargetsBottomPRToDefaultBranch(t *testing.T) { restore := git.SetOps(&git.MockOps{ GitDirFn: func() (string, error) { return "", fmt.Errorf("not a git repository") }, BranchExistsFn: func(string) (bool, error) { return false, nil }, - DefaultBranchFn: func() (string, error) { + DefaultBranchFn: func(string) (string, error) { defaultBranchCalled = true return "develop", nil }, @@ -1477,7 +1477,7 @@ func TestLink_DefaultBase_CreatesBottomPROnDefaultBranch(t *testing.T) { BranchExistsFn: func(name string) (bool, error) { return name == "feat-a" || name == "feat-b", nil }, PushFn: func(string, []string, bool, bool) error { return nil }, ResolveRemoteFn: func(string) (string, error) { return "origin", nil }, - DefaultBranchFn: func() (string, error) { return "develop", nil }, + DefaultBranchFn: func(string) (string, error) { return "develop", nil }, }) defer restore() @@ -1521,7 +1521,7 @@ func TestLink_DefaultBase_ErrorWhenUnresolvable(t *testing.T) { restore := git.SetOps(&git.MockOps{ GitDirFn: func() (string, error) { return "", fmt.Errorf("not a git repository") }, BranchExistsFn: func(string) (bool, error) { return false, nil }, - DefaultBranchFn: func() (string, error) { return "", fmt.Errorf("no default branch") }, + DefaultBranchFn: func(string) (string, error) { return "", fmt.Errorf("no default branch") }, }) defer restore() @@ -1556,7 +1556,7 @@ func TestLink_ExplicitBase_SkipsDefaultBranchResolution(t *testing.T) { restore := git.SetOps(&git.MockOps{ GitDirFn: func() (string, error) { return "", fmt.Errorf("not a git repository") }, BranchExistsFn: func(string) (bool, error) { return false, nil }, - DefaultBranchFn: func() (string, error) { + DefaultBranchFn: func(string) (string, error) { defaultBranchCalled = true return "develop", nil }, diff --git a/cmd/utils.go b/cmd/utils.go index db43aa9..ab27d3b 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -1923,6 +1923,38 @@ func ensureRerere(cfg *config.Config) error { return nil } +// resolveDefaultBranch finds the repository's default branch without prompting. +// It asks git about the configured remote (falling back to origin), then the +// GitHub API, and finally guesses a local main or master branch. +func resolveDefaultBranch(cfg *config.Config, remoteOverride string) (string, error) { + remote := remoteOverride + if remote == "" { + remote = "origin" + branch, _ := git.CurrentBranch() + if resolved, err := git.ResolveRemote(branch); err == nil { + remote = resolved + } + } + if name, err := git.DefaultBranch(remote); err == nil && name != "" { + return name, nil + } + if client, err := cfg.GitHubClient(); err == nil { + if name, err := client.DefaultBranch(); err == nil && name != "" { + return name, nil + } + } + for _, name := range []string{"main", "master"} { + exists, err := git.BranchExists(name) + if err != nil { + return "", err + } + if exists { + return name, nil + } + } + return "", git.ErrNoDefaultBranch +} + // pickRemote determines which remote to use. If remoteOverride is // non-empty, it is returned directly. Otherwise it delegates to // git.ResolveRemote for config-based resolution and remote listing. diff --git a/cmd/utils_test.go b/cmd/utils_test.go index f421188..e67ee70 100644 --- a/cmd/utils_test.go +++ b/cmd/utils_test.go @@ -6,6 +6,7 @@ import ( "io" "os" "path/filepath" + "slices" "strings" "testing" @@ -2057,6 +2058,102 @@ func TestUpdateBaseSHAsPreservesLastValidBase(t *testing.T) { // TestFullScreenCanvas verifies that the full-screen TUIs paint the GitHub // canvas only on a color-enabled truecolor terminal, and honor the // GH_STACK_BACKGROUND opt-out. +func TestResolveDefaultBranch(t *testing.T) { + gitErr := errors.New("unknown") + for _, tt := range []struct { + name string + override string + resolveRemote func(string) (string, error) + gitDefault map[string]string + apiDefault string + apiErr error + localBranches []string + want string + wantErr error + wantRemoteUsed string + }{ + { + name: "resolved remote", + resolveRemote: func(string) (string, error) { return "upstream", nil }, + gitDefault: map[string]string{"upstream": "develop"}, + want: "develop", + wantRemoteUsed: "upstream", + }, + { + name: "override wins", + override: "fork", + gitDefault: map[string]string{"fork": "trunk"}, + want: "trunk", + wantRemoteUsed: "fork", + }, + { + name: "origin when remote is ambiguous", + resolveRemote: func(string) (string, error) { return "", &git.ErrMultipleRemotes{Remotes: []string{"a", "b"}} }, + gitDefault: map[string]string{"origin": "develop"}, + want: "develop", + wantRemoteUsed: "origin", + }, + { + name: "GitHub API when git cannot tell", + apiDefault: "release", + localBranches: []string{"main"}, + want: "release", + }, + { + name: "local main as a last resort", + apiErr: gitErr, + localBranches: []string{"master", "main"}, + want: "main", + }, + { + name: "local master as a last resort", + localBranches: []string{"master"}, + want: "master", + }, + { + name: "nothing found", + wantErr: git.ErrNoDefaultBranch, + }, + } { + t.Run(tt.name, func(t *testing.T) { + resolveRemote := tt.resolveRemote + if resolveRemote == nil { + resolveRemote = func(string) (string, error) { return "origin", nil } + } + var remoteUsed string + restore := git.SetOps(&git.MockOps{ + ResolveRemoteFn: resolveRemote, + DefaultBranchFn: func(remote string) (string, error) { + remoteUsed = remote + if name, ok := tt.gitDefault[remote]; ok { + return name, nil + } + return "", git.ErrNoDefaultBranch + }, + BranchExistsFn: func(name string) (bool, error) { + return slices.Contains(tt.localBranches, name), nil + }, + }) + defer restore() + cfg, _, _ := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + DefaultBranchFn: func() (string, error) { return tt.apiDefault, tt.apiErr }, + } + + got, err := resolveDefaultBranch(cfg, tt.override) + if tt.wantErr != nil { + assert.ErrorIs(t, err, tt.wantErr) + return + } + require.NoError(t, err) + assert.Equal(t, tt.want, got) + if tt.wantRemoteUsed != "" { + assert.Equal(t, tt.wantRemoteUsed, remoteUsed) + } + }) + } +} + func TestFullScreenCanvas(t *testing.T) { tests := []struct { name string diff --git a/internal/git/git.go b/internal/git/git.go index 37bd7c4..54e22c9 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -297,9 +297,14 @@ func FetchBranches(remote string, branches []string) error { return ops.FetchBranches(remote, branches) } -// DefaultBranch returns the HEAD branch from origin. -func DefaultBranch() (string, error) { - return ops.DefaultBranch() +// ErrNoDefaultBranch reports that a remote's default branch is unknown. +var ErrNoDefaultBranch = errors.New("could not determine default branch") + +// DefaultBranch returns the remote's default branch from the local +// refs/remotes//HEAD pointer, or by asking the remote when that +// pointer is missing. +func DefaultBranch(remote string) (string, error) { + return ops.DefaultBranch(remote) } // CreateBranch creates a new branch from the given base. diff --git a/internal/git/gitops.go b/internal/git/gitops.go index 0dd9a96..b09204f 100644 --- a/internal/git/gitops.go +++ b/internal/git/gitops.go @@ -45,7 +45,7 @@ type Ops interface { Fetch(remote string) error FetchBranch(remote, branch string) error FetchBranches(remote string, branches []string) error - DefaultBranch() (string, error) + DefaultBranch(remote string) (string, error) CreateBranch(name, base string) error Push(remote string, branches []string, force, atomic bool) error ResolveRemote(branch string) (string, error) @@ -230,25 +230,47 @@ func isMissingRemoteRefError(err error) bool { return err != nil && strings.Contains(err.Error(), "couldn't find remote ref") } -func (d *defaultOps) DefaultBranch() (string, error) { - ref, err := d.run("symbolic-ref", "--quiet", "refs/remotes/origin/HEAD") +func (d *defaultOps) DefaultBranch(remote string) (string, error) { + prefix := "refs/remotes/" + remote + "/" + name := prefix+"HEAD" + ref, err := d.run("symbolic-ref", "--quiet", name) + + if err == nil { + return strings.TrimPrefix(ref, prefix), nil + } + + var gitErr *cligit.GitError + if !errors.As(err, &gitErr) || gitErr.ExitCode != 1 || gitErr.Stderr != "" { + return "", err + } + + // The local pointer is missing, e.g. when the remote was added to an + // existing repository instead of cloned, so ask the remote directly. + cmd, err := d.command("ls-remote", "--symref", remote, "HEAD") if err != nil { - var gitErr *cligit.GitError - if !errors.As(err, &gitErr) || gitErr.ExitCode != 1 || gitErr.Stderr != "" { - return "", err + return "", err + } + + cmd.Env = append(cmd.Environ(), "GIT_TERMINAL_PROMPT=0") + if out, err := cmd.Output(); err == nil { + if branch := parseSymrefHead(string(out)); branch != "" { + return branch, nil } - for _, name := range []string{"main", "master"} { - exists, lookupErr := d.BranchExists(name) - if lookupErr != nil { - return "", lookupErr - } - if exists { - return name, nil - } + } + + return "", fmt.Errorf("%w for remote %q", ErrNoDefaultBranch, remote) +} + +// parseSymrefHead extracts the branch from `git ls-remote --symref` output, +// whose first line is "ref: refs/heads/\tHEAD". +func parseSymrefHead(out string) string { + for _, line := range strings.Split(out, "\n") { + fields := strings.Fields(line) + if len(fields) == 3 && fields[0] == "ref:" && fields[2] == "HEAD" { + return strings.TrimPrefix(fields[1], "refs/heads/") } - return "", err } - return strings.TrimPrefix(ref, "refs/remotes/origin/"), nil + return "" } func (d *defaultOps) CreateBranch(name, base string) error { diff --git a/internal/git/gitops_test.go b/internal/git/gitops_test.go index cb1822c..4b0b9fc 100644 --- a/internal/git/gitops_test.go +++ b/internal/git/gitops_test.go @@ -678,6 +678,32 @@ func TestIntegration_ResolveRemote_GitPushConfigTakesPrecedence(t *testing.T) { assert.Equal(t, "origin", remote) } +func TestIntegration_DefaultBranch(t *testing.T) { + bareDir, cloneDir := setupBareAndClone(t) + gitExec(t, cloneDir, "push", "origin", "main:develop") + gitExec(t, bareDir, "-c", "safe.bareRepository=all", "symbolic-ref", "HEAD", "refs/heads/develop") + + dir := setupLocalRepo(t) + gitExec(t, dir, "remote", "add", "upstream", bareDir) + restoreDir := withGitDir(t, dir) + defer restoreDir() + + branch, err := DefaultBranch("upstream") + require.NoError(t, err) + assert.Equal(t, "develop", branch, "asks the remote when no local pointer exists") + _, err = gitExecMayFail(t, dir, "symbolic-ref", "refs/remotes/upstream/HEAD") + assert.Error(t, err, "querying the remote must not write a local pointer") + + gitExec(t, dir, "update-ref", "refs/remotes/upstream/cached", "HEAD") + gitExec(t, dir, "symbolic-ref", "refs/remotes/upstream/HEAD", "refs/remotes/upstream/cached") + branch, err = DefaultBranch("upstream") + require.NoError(t, err) + assert.Equal(t, "cached", branch, "the local pointer avoids a network call") + + _, err = DefaultBranch("missing") + assert.ErrorIs(t, err, ErrNoDefaultBranch) +} + func TestIntegration_SaveAndGetRemote(t *testing.T) { _, cloneDir := setupBareAndClone(t) restoreDir := withGitDir(t, cloneDir) @@ -1755,14 +1781,13 @@ func TestIntegration_WorktreeLocalMutations(t *testing.T) { linked, path := addTestWorktree(t, root, dir, "feature") restore := forbidGlobalWorktreeQueries(t) defer restore() - defaultBranch, err := linked.DefaultBranch() - require.NoError(t, err) - assert.Equal(t, "main", defaultBranch) + _, err := linked.DefaultBranch("origin") + require.ErrorIs(t, err, ErrNoDefaultBranch, "local branch names are not guessed at the git layer") gitExec(t, dir, "update-ref", "refs/remotes/origin/remote-trunk", "main") gitExec(t, dir, "symbolic-ref", "refs/remotes/origin/HEAD", "refs/remotes/origin/remote-trunk") - defaultBranch, err = linked.DefaultBranch() + defaultBranch, err := linked.DefaultBranch("origin") require.NoError(t, err) - assert.Equal(t, "remote-trunk", defaultBranch, "a remote HEAD takes precedence over local branch-name fallbacks") + assert.Equal(t, "remote-trunk", defaultBranch) initial := gitExec(t, dir, "rev-parse", "HEAD") writeFile(t, dir, "new.txt", "new main commit\n") gitExec(t, dir, "add", ".") diff --git a/internal/git/mock_ops.go b/internal/git/mock_ops.go index a3f894c..716a213 100644 --- a/internal/git/mock_ops.go +++ b/internal/git/mock_ops.go @@ -22,7 +22,7 @@ type MockOps struct { FetchFn func(string) error FetchBranchFn func(string, string) error FetchBranchesFn func(string, []string) error - DefaultBranchFn func() (string, error) + DefaultBranchFn func(string) (string, error) CreateBranchFn func(string, string) error PushFn func(string, []string, bool, bool) error ResolveRemoteFn func(string) (string, error) @@ -160,9 +160,9 @@ func (m *MockOps) FetchBranches(remote string, branches []string) error { return nil } -func (m *MockOps) DefaultBranch() (string, error) { +func (m *MockOps) DefaultBranch(remote string) (string, error) { if m.DefaultBranchFn != nil { - return m.DefaultBranchFn() + return m.DefaultBranchFn(remote) } return "main", nil } diff --git a/internal/github/client_interface.go b/internal/github/client_interface.go index c1281ed..7b98bf6 100644 --- a/internal/github/client_interface.go +++ b/internal/github/client_interface.go @@ -22,6 +22,7 @@ type ClientOps interface { GetAsyncMergeResult(prNumber int, uuid string) (*AsyncMergeResult, error) PRTitles(numbers []int) (map[int]string, error) BaseBranchUsesMergeQueue(baseRef string) (bool, error) + DefaultBranch() (string, error) } // Compile-time check that Client satisfies ClientOps. diff --git a/internal/github/github.go b/internal/github/github.go index 88d3f6f..bc01a66 100644 --- a/internal/github/github.go +++ b/internal/github/github.go @@ -546,6 +546,17 @@ func (c *Client) FindStackForPR(prNumber int) (*RemoteStack, error) { return &stacks[0], nil } +// DefaultBranch returns the repository's default branch. +func (c *Client) DefaultBranch() (string, error) { + var repo struct { + DefaultBranch string `json:"default_branch"` + } + if err := c.rest.Get(fmt.Sprintf("repos/%s/%s", c.owner, c.repo), &repo); err != nil { + return "", fmt.Errorf("fetching repository: %w", err) + } + return repo.DefaultBranch, nil +} + // GetStack fetches a single stack by its stack number. func (c *Client) GetStack(stackNumber int) (*RemoteStack, error) { path := fmt.Sprintf("repos/%s/%s/stacks/%d", c.owner, c.repo, stackNumber) diff --git a/internal/github/mock_client.go b/internal/github/mock_client.go index a31dd4e..26575a3 100644 --- a/internal/github/mock_client.go +++ b/internal/github/mock_client.go @@ -22,6 +22,7 @@ type MockClient struct { GetAsyncMergeResultFn func(int, string) (*AsyncMergeResult, error) PRTitlesFn func([]int) (map[int]string, error) BaseBranchUsesMergeQueueFn func(string) (bool, error) + DefaultBranchFn func() (string, error) } // Compile-time check that MockClient satisfies ClientOps. @@ -170,3 +171,10 @@ func (m *MockClient) BaseBranchUsesMergeQueue(baseRef string) (bool, error) { } return false, nil } + +func (m *MockClient) DefaultBranch() (string, error) { + if m.DefaultBranchFn != nil { + return m.DefaultBranchFn() + } + return "", nil +}