Skip to content

Fix hardcoded main branch in gh aw trial --host-repo #43405

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
pelikhan merged 8 commits into from
Jul 5, 2026
Merged

Fix hardcoded main branch in gh aw trial --host-repo #43405

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8aee340
Initial plan
Copilot Jul 4, 2026
e6ed69e
Initial plan for fixing hardcoded main branch
Copilot Jul 4, 2026
980047c
Fix hardcoded 'main' branch in gh aw trial --host-repo
Copilot Jul 4, 2026
bedf6c5
Address code review feedback: add cmd.Dir and debug logging
Copilot Jul 4, 2026
18342aa
Replace getLocalBranch with getCurrentBranchIn built on existing helper
Copilot Jul 5, 2026
d9331c0
docs(adr): add draft ADR-43405 for dynamic default-branch detection i…
github-actions[bot] Jul 5, 2026
c586c11
Merge branch 'main' into copilot/fix-gh-aw-trial-default-branch
github-actions[bot] Jul 5, 2026
b37a1d1
fix: use --rebase for git pull in trial flows and add detached-HEAD test
Copilot Jul 5, 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
17 changes: 15 additions & 2 deletions pkg/cli/git.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
17 changes: 12 additions & 5 deletions pkg/cli/trial_helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"

@github-actions github-actions Bot Jul 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Silent "main" fallback reintroduces the exact bug this PR is fixing when branch detection fails.

💡 Details and suggested fix

If getCurrentBranchIn returns an error — including the detached HEAD path where git branch --show-current exits 0 with empty output — execution silently falls through to branch = "main". The error is only logged at debug level; the function then proceeds to git push origin main, silently pushing to the wrong branch in non-main-default repos.

The tempDir is a fresh clone, making detached HEAD uncommon in practice, but any transient git failure (filesystem permissions, corrupt index) will also trigger this path and produce a misleading SUCCESS log for the user.

Suggested approach: propagate the error rather than swallowing it:

branch, err := getCurrentBranchIn(tempDir)
if err != nil {
    return fmt.Errorf("failed to detect branch for push in %s: %w", tempDir, err)
}

If a graceful fallback is truly desired, at minimum surface it to the user via console.FormatWarningMessage rather than a silent debug log, so operators know the wrong branch may be in use.

}
// 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", "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))
}
Comment on lines +372 to 380

// 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))
}
Expand Down
14 changes: 10 additions & 4 deletions pkg/cli/trial_repository.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"

@github-actions github-actions Bot Jul 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same silent "main" fallback here — identical correctness risk as in trial_helpers.go.

💡 Details and suggested fix

When getCurrentBranchIn fails (including an empty-output detached HEAD result), branch is silently set to "main" and git push origin main proceeds without any user-visible warning. On a repo whose default branch is trunk or develop, this pushes workflow files to the wrong branch and logs a green success message.

Because tempDir is a freshly cloned host repo, detached HEAD is unlikely in the happy path, but the silent recovery behavior makes failures invisible. The caller of commitAndPushWorkflow propagates its error, so failing loudly here is the right call:

branch, err := getCurrentBranchIn(tempDir)
if err != nil {
    return fmt.Errorf("failed to detect branch for push in %s: %w", tempDir, err)
}

}
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", "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))
}
Comment on lines +531 to 537

// 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))
Expand Down
69 changes: 68 additions & 1 deletion pkg/cli/trial_repository_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@

package cli

import "testing"
import (
"os/exec"
"testing"
)

func TestTrialRepositoryURLHelpers(t *testing.T) {
tests := []struct {
Expand Down Expand Up @@ -61,3 +64,67 @@ func TestTrialRepositoryURLHelpers(t *testing.T) {
})
}
}

func TestGetCurrentBranchIn(t *testing.T) {

@github-actions github-actions Bot Jul 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The TestGetCurrentBranchIn test exercises getCurrentBranchIn, which is defined in pkg/cli/git.go. The existing TestGetCurrentBranch / TestGetCurrentBranchNotInRepo tests for the same function family already live in pkg/cli/git_test.go.

Consider moving this test there to keep all getCurrentBranch* coverage in one discoverable place and align with the established convention.

@copilot please address this.

// 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")

@github-actions github-actions Bot Jul 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test covers non-git-directory error but not detached HEAD, which is the primary trigger for the branch="main" fallback in both callers.

💡 Suggested additional test case

git branch --show-current returns exit code 0 with empty output in a detached HEAD state. getCurrentBranchIn converts that to an errors.New("could not determine current branch") — the separate empty-string check — but this path is completely untested.

This matters because both commitAndPushWorkflow and copyTrialResultsToHostRepo fall back to "main" on any error, and the detached HEAD path is the most operationally relevant way that error arises. Without this test, the test suite gives false confidence that error handling for branch detection is complete.

t.Run("returns error for detached HEAD", func(t *testing.T) {
    dir := t.TempDir()
    initRepo(t, dir, "main")
    // detach HEAD by checking out the commit SHA directly
    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)
        }
    }
    // Get current HEAD SHA and detach
    sha, _ := exec.Command("git", "-C", dir, "rev-parse", "HEAD").Output()
    run("git", "checkout", strings.TrimSpace(string(sha)))

    _, err := getCurrentBranchIn(dir)
    if err == nil {
        t.Fatal("getCurrentBranchIn() expected an error for detached HEAD, got nil")
    }
})

}
})
Comment on lines +112 to +130
}
Loading