fix: add-wizard ls-remote default branch parsing bug#24105
Conversation
The ls-remote fallback in updateLocalBranch() was using strings.Fields
to split the line 'ref: refs/heads/main<TAB>abc123', which produces
['ref:', 'refs/heads/main', 'abc123']. The code then used parts[0]
('ref:') and tried to TrimPrefix 'ref: refs/heads/' from it — which
didn't match, leaving defaultBranch set to 'ref:'. This caused
'git fetch origin ref:' to fail with 'couldn't find remote ref ref'.
Fix: Extract parseDefaultBranchFromLsRemote() into a standalone
testable function that splits on tab first (matching actual git output
format), then trims the 'ref: refs/heads/' prefix correctly.
Add comprehensive tests including real-git integration tests that
create actual repos and verify parsing against real git ls-remote
output, so this bug cannot regress again.
There was a problem hiding this comment.
Pull request overview
Fixes a recurring failure in gh aw add-wizard where updating the local branch after a PR merge could attempt to fetch an invalid ref due to incorrect parsing of git ls-remote --symref origin HEAD.
Changes:
- Replaces inline
git ls-remote --symrefparsing with a dedicatedparseDefaultBranchFromLsRemote()helper. - Updates parsing to split on tab (matching git output) and extract the branch name correctly.
- Adds unit tests plus “real git” repository-based tests to prevent regressions.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/cli/add_interactive_git.go | Extracts and uses a robust parser for default-branch detection from git ls-remote --symref output. |
| pkg/cli/add_interactive_git_test.go | Adds parser regression/unit tests and repo-backed tests that exercise real git ls-remote --symref output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // | ||
| // Example output: | ||
| // | ||
| // ref: refs/heads/main abc123 |
There was a problem hiding this comment.
The example output in this doc comment doesn’t match actual git ls-remote --symref origin HEAD output: the symref line’s second field is HEAD (not a commit SHA), and the SHA appears on the subsequent line. Updating the example will avoid misleading future maintainers and test authors.
| // ref: refs/heads/main abc123 | |
| // ref: refs/heads/main HEAD |
| // Create a bare repo to serve as "origin" | ||
| bareDir := t.TempDir() | ||
| runGit(t, bareDir, "init", "--bare", "--initial-branch="+tt.defaultBranch) | ||
|
|
||
| // Clone it to create a working repo with that origin | ||
| workDir := filepath.Join(t.TempDir(), "work") | ||
| runGitIn(t, "", "clone", bareDir, workDir) |
There was a problem hiding this comment.
This test relies on git init --initial-branch=..., which requires relatively recent Git versions; on older Git it will fail and cause unit tests to fail. Consider avoiding --initial-branch by initializing normally and then setting the bare repo’s HEAD to the desired branch (e.g., via git symbolic-ref HEAD refs/heads/<branch> after creating/pushing the branch), or skipping when git init returns an error indicating the flag is unsupported.
|
Js failure unrelated. @copilot review comments |
Addressed both review comments:
Note: the push is currently blocked by a repository size rule (large video files in the shallow clone's base commit history) — this appears to be an environment issue unrelated to the code changes. |
Problem
gh aw add-wizardfails to pull merged changes after PR merge with:This has been a recurring bug — attempted to fix 5 times and it kept coming back.
Root Cause
The
updateLocalBranch()fallback parsesgit ls-remote --symref origin HEADoutput, which looks like:The code used
strings.Fields()to split this, producing["ref:", "refs/heads/main", "abc123"]. It then calledTrimPrefix(parts[0], "ref: refs/heads/")— butparts[0]is just"ref:", so the prefix doesn't match, leavingdefaultBranch = "ref:". This causedgit fetch origin ref:to fail.Fix
parseDefaultBranchFromLsRemote()into a standalone testable functionresult must not start with "ref:")git ls-remote --symref, and verify the parser works against actual git outputThe real-git tests ensure this specific bug cannot regress again — any future change that breaks the parsing will fail against actual git output, not just synthetic strings.