diff --git a/docs/adr/43405-dynamic-branch-detection-for-gh-aw-trial.md b/docs/adr/43405-dynamic-branch-detection-for-gh-aw-trial.md new file mode 100644 index 00000000000..2dda0462b53 --- /dev/null +++ b/docs/adr/43405-dynamic-branch-detection-for-gh-aw-trial.md @@ -0,0 +1,55 @@ +# ADR-43405: Dynamic Default-Branch Detection for `gh aw trial --host-repo` + +**Date**: 2026-07-05 +**Status**: Draft +**Deciders**: pelikhan, copilot-swe-agent + +--- + +### Context + +`gh aw trial --host-repo=.` fails immediately when the host repository's default branch is not `main`. The two internal helpers `commitAndPushWorkflow` and `copyTrialResultsToHostRepo` unconditionally ran `git pull origin main` and `git push origin main`, which do not exist in repositories that use `master`, `trunk`, or any other default branch name. Additionally, neither helper set `cmd.Dir` on the `git pull` command, which was a pre-existing bug that could silently operate on the wrong directory. The tool needs to work correctly across all repositories regardless of their default branch name. + +### Decision + +We will introduce `getCurrentBranchIn(dir string)` — a directory-aware variant of the existing `getCurrentBranch()` helper — and use it in both `commitAndPushWorkflow` and `copyTrialResultsToHostRepo` to read the actual default branch from the locally-checked-out clone (`tempDir`) at runtime. When detection fails (non-git directory, detached HEAD, command error), the implementation falls back to `"main"` with debug logging. Since `tempDir` is always a fresh clone of the host repo, `HEAD` reliably reflects the remote default branch. + +### Alternatives Considered + +#### Alternative 1: Explicit `--default-branch` CLI flag + +Users could pass the default branch name as a flag to `gh aw trial`. This avoids any dynamic detection and makes the branch explicit. + +This was not chosen because it places an unnecessary burden on every user to know their repo's default branch name. The local clone already carries this information, making auto-detection both simpler and more reliable. A flag would also break automation scripts that work across repos with different default branches. + +#### Alternative 2: Query the remote for the default branch + +The default branch could be queried from the remote via `git remote show origin` (parsing `HEAD branch:`) or the GitHub API. This would work regardless of local checkout state. + +This was not chosen because `tempDir` is always a fresh clone of the host repo with a checked-out `HEAD`, so reading `HEAD` locally is equivalent to reading the remote default branch — while being faster, simpler, and not requiring network access or API credentials at that point in the workflow. + +#### Alternative 3: Read default branch from GitHub API / repository metadata + +The `gh` CLI or GitHub API could provide the default branch before cloning. This is authoritative but adds an API call and couples the implementation to the GitHub API. + +This was not chosen for the same reason as Alternative 2: the local clone already reflects the remote's `HEAD`, and the added complexity is not justified for a simple bug fix. + +### Consequences + +#### Positive +- `gh aw trial --host-repo=.` now works correctly for repositories using `master`, `trunk`, or any other default branch name. +- Reuses the existing `getCurrentBranch()` infrastructure, keeping the codebase DRY; the new `getCurrentBranchIn(dir)` function is the single source of truth for branch detection with or without a directory argument. +- Adds comprehensive test coverage (`TestGetCurrentBranchIn`) for `main`, `master`, custom branch names, and the non-git-directory error path. +- Fixes the pre-existing missing `cmd.Dir` on `git pull` calls, preventing potential silent operation on the wrong working directory. + +#### Negative +- If `tempDir` is somehow in detached HEAD state (edge case: shallow clone, CI runner behavior), the fallback to `"main"` will be used silently (only debug-logged), which could cause a push failure or push to an unintended branch without a user-visible error message. +- Relies on local checkout state (`HEAD`) rather than an authoritative remote query; if the remote default branch changes between clone and push (extremely unlikely in normal usage), the push could target a stale branch name. + +#### Neutral +- The `getCurrentBranch()` function (no-dir variant) is refactored to delegate to `getCurrentBranchIn("")`, maintaining backward compatibility with all existing callers. +- The fallback branch name remains `"main"` for compatibility with the majority of GitHub repositories. + +--- + +*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* diff --git a/pkg/cli/git.go b/pkg/cli/git.go index d0b860c9ad6..c6cc712d252 100644 --- a/pkg/cli/git.go +++ b/pkg/cli/git.go @@ -441,10 +441,23 @@ func ensureLogsGitignore() error { return nil } -// getCurrentBranch gets the current git branch name +// getCurrentBranch gets the current git branch name in the current working directory. func getCurrentBranch() (string, error) { - gitLog.Print("Getting current git branch") + return getCurrentBranchIn("") +} + +// getCurrentBranchIn gets the current git branch name in the given directory. +// Pass an empty string to use the current working directory. +func getCurrentBranchIn(dir string) (string, error) { + if dir == "" { + gitLog.Print("Getting current git branch") + } else { + gitLog.Printf("Getting current git branch in %s", dir) + } cmd := exec.Command("git", "branch", "--show-current") + if dir != "" { + cmd.Dir = dir + } output, err := cmd.Output() if err != nil { gitLog.Printf("Failed to get current branch: %v", err) diff --git a/pkg/cli/trial_helpers.go b/pkg/cli/trial_helpers.go index cf8baa62f73..fb213cb9a83 100644 --- a/pkg/cli/trial_helpers.go +++ b/pkg/cli/trial_helpers.go @@ -364,17 +364,24 @@ func copyTrialResultsToHostRepo(tempDir, dateTimeID string, workflowNames []stri return fmt.Errorf("failed to commit trial results: %w (output: %s)", err, string(output)) } - // Pull latest changes from main before pushing to avoid conflicts + branch, err := getCurrentBranchIn(tempDir) + if err != nil { + trialLog.Printf("Failed to detect branch in %s: %v, falling back to main", tempDir, err) + branch = "main" + } + // Pull latest changes before pushing to avoid conflicts if verbose { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Pulling latest changes from main branch")) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Pulling latest changes from "+branch+" branch")) } - cmd = exec.Command("git", "pull", "origin", "main") + cmd = exec.Command("git", "pull", "--rebase", "origin", branch) + cmd.Dir = tempDir if output, err := cmd.CombinedOutput(); err != nil { return fmt.Errorf("failed to pull latest changes: %w (output: %s)", err, string(output)) } - // Push to main - cmd = exec.Command("git", "push", "origin", "main") + // Push to current branch + cmd = exec.Command("git", "push", "origin", branch) + cmd.Dir = tempDir if output, err := cmd.CombinedOutput(); err != nil { return fmt.Errorf("failed to push trial results: %w (output: %s)", err, string(output)) } diff --git a/pkg/cli/trial_repository.go b/pkg/cli/trial_repository.go index 7bfcbcfdf0f..2aafd7be8b7 100644 --- a/pkg/cli/trial_repository.go +++ b/pkg/cli/trial_repository.go @@ -522,16 +522,22 @@ func commitAndPushWorkflow(tempDir, workflowName string, verbose bool) error { return fmt.Errorf("failed to commit changes: %w (output: %s)", err, string(output)) } + branch, err := getCurrentBranchIn(tempDir) + if err != nil { + trialRepoLog.Printf("Failed to detect branch in %s: %v, falling back to main", tempDir, err) + branch = "main" + } if verbose { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Pulling latest changes from main branch")) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Pulling latest changes from "+branch+" branch")) } - cmd = exec.Command("git", "pull", "origin", "main") + cmd = exec.Command("git", "pull", "--rebase", "origin", branch) + cmd.Dir = tempDir if output, err := cmd.CombinedOutput(); err != nil { return fmt.Errorf("failed to pull latest changes: %w (output: %s)", err, string(output)) } - // Push to main - cmd = exec.Command("git", "push", "origin", "main") + // Push to current branch + cmd = exec.Command("git", "push", "origin", branch) cmd.Dir = tempDir if output, err := cmd.CombinedOutput(); err != nil { return fmt.Errorf("failed to push changes: %w (output: %s)", err, string(output)) diff --git a/pkg/cli/trial_repository_test.go b/pkg/cli/trial_repository_test.go index 488c5994465..37f8e289378 100644 --- a/pkg/cli/trial_repository_test.go +++ b/pkg/cli/trial_repository_test.go @@ -2,7 +2,11 @@ package cli -import "testing" +import ( + "os/exec" + "strings" + "testing" +) func TestTrialRepositoryURLHelpers(t *testing.T) { tests := []struct { @@ -61,3 +65,89 @@ func TestTrialRepositoryURLHelpers(t *testing.T) { }) } } + +func TestGetCurrentBranchIn(t *testing.T) { + // initRepo creates a minimal git repo in dir with the given branch name. + initRepo := func(t *testing.T, dir, branch string) { + t.Helper() + run := func(args ...string) { + t.Helper() + cmd := exec.Command(args[0], args[1:]...) + cmd.Dir = dir + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("command %v failed: %v (output: %s)", args, err, out) + } + } + run("git", "init") + run("git", "config", "user.email", "test@example.com") + run("git", "config", "user.name", "Test") + run("git", "symbolic-ref", "HEAD", "refs/heads/"+branch) + run("git", "commit", "--allow-empty", "-m", "init") + } + + t.Run("returns main for a repo using main", func(t *testing.T) { + dir := t.TempDir() + initRepo(t, dir, "main") + got, err := getCurrentBranchIn(dir) + if err != nil { + t.Fatalf("getCurrentBranchIn() unexpected error: %v", err) + } + if got != "main" { + t.Fatalf("getCurrentBranchIn() = %q, want %q", got, "main") + } + }) + + t.Run("returns master for a repo using master", func(t *testing.T) { + dir := t.TempDir() + initRepo(t, dir, "master") + got, err := getCurrentBranchIn(dir) + if err != nil { + t.Fatalf("getCurrentBranchIn() unexpected error: %v", err) + } + if got != "master" { + t.Fatalf("getCurrentBranchIn() = %q, want %q", got, "master") + } + }) + + t.Run("returns custom branch name", func(t *testing.T) { + dir := t.TempDir() + initRepo(t, dir, "trunk") + got, err := getCurrentBranchIn(dir) + if err != nil { + t.Fatalf("getCurrentBranchIn() unexpected error: %v", err) + } + if got != "trunk" { + t.Fatalf("getCurrentBranchIn() = %q, want %q", got, "trunk") + } + }) + + t.Run("returns error for non-git directory", func(t *testing.T) { + dir := t.TempDir() + _, err := getCurrentBranchIn(dir) + if err == nil { + t.Fatal("getCurrentBranchIn() expected an error for non-git directory, got nil") + } + }) + + t.Run("returns error in detached HEAD state", func(t *testing.T) { + dir := t.TempDir() + initRepo(t, dir, "main") + // Detach HEAD by checking out the commit hash directly. + cmd := exec.Command("git", "rev-parse", "HEAD") + cmd.Dir = dir + out, err := cmd.Output() + if err != nil { + t.Fatalf("git rev-parse HEAD failed: %v", err) + } + hash := strings.TrimSpace(string(out)) + detach := exec.Command("git", "checkout", hash) + detach.Dir = dir + if out, err := detach.CombinedOutput(); err != nil { + t.Fatalf("git checkout %s failed: %v (output: %s)", hash, err, out) + } + _, err = getCurrentBranchIn(dir) + if err == nil { + t.Fatal("getCurrentBranchIn() expected an error in detached HEAD state, got nil") + } + }) +}