diff --git a/gitea/prs.go b/gitea/prs.go index a0920ae..8cc0106 100644 --- a/gitea/prs.go +++ b/gitea/prs.go @@ -58,6 +58,10 @@ func convertGiteaPR(pr *gitea.PullRequest) forge.PullRequest { } if pr.Head != nil { result.Head = forge.PRBranch{ + // Ref is the head branch name when one exists, but Gitea/Forgejo + // fall back to a refs/pull//head ref when there is no head + // branch: AGit-flow PRs (pushed to refs/for/*, never branched) or + // PRs whose branch was deleted. Callers must tolerate a full ref. Ref: pr.Head.Ref, SHA: pr.Head.Sha, } diff --git a/internal/cli/pr.go b/internal/cli/pr.go index 31c7da7..a5bd7e2 100644 --- a/internal/cli/pr.go +++ b/internal/cli/pr.go @@ -601,16 +601,22 @@ The argument can be a PR number or a full URL: return fmt.Errorf("getting PR #%d: %w", number, err) } - // remoteRef is the branch name on the remote (PR's head branch) + // remoteRef is usually the PR's head branch name, but Gitea/Forgejo + // report a refs/pull//head ref when there is no head branch + // (AGit-flow PRs, or PRs whose branch was deleted). Such a ref only + // lives on the base repo. remoteRef := pr.Head.Ref - // localBranch is what we'll name the local branch (defaults to remote ref) - localBranch := remoteRef - if flagBranch != "" { - localBranch = flagBranch + // localBranch is what we'll name the local branch (defaults to the + // head branch name, or pr- when only a pull ref is known). + localBranch := flagBranch + if localBranch == "" { + localBranch = defaultLocalBranch(pr) } - if pr.Head.Fork != nil { + // A pull ref isn't present on the fork remote, only on origin, so + // route it through the same-repo path even for fork PRs. + if pr.Head.Fork != nil && !isFullRef(remoteRef) { return checkoutForkPR(ctx, domain, pr, remoteRef, localBranch, flagRemoteName, flagDetach, flagForce) } @@ -699,8 +705,31 @@ func remoteMatches(existingURL, wantURL string) bool { return domain == wantDomain && owner == wantOwner && repo == wantRepo } +// isFullRef reports whether ref is a fully-qualified git ref (e.g. +// refs/pull//head) rather than a bare branch name. +func isFullRef(ref string) bool { + return strings.HasPrefix(ref, "refs/") +} + +// defaultLocalBranch picks the local branch name for a checked-out PR. It uses +// the head branch name when available, but falls back to pr- when only +// a pull ref is known (Gitea/Forgejo PRs with no head branch, e.g. AGit flow). +func defaultLocalBranch(pr *forges.PullRequest) string { + if isFullRef(pr.Head.Ref) { + return fmt.Sprintf("pr-%d", pr.Number) + } + return pr.Head.Ref +} + func gitCheckout(ctx context.Context, remote, remoteRef, localBranch string, detach, force bool) error { - refspec := fmt.Sprintf("+refs/heads/%s:refs/remotes/%s/%s", remoteRef, remote, remoteRef) + // A bare branch name needs the refs/heads/ prefix; a full ref (e.g. + // refs/pull//head) is fetched as-is. The remote-tracking ref is named + // after localBranch so a pull ref doesn't leak into refs/remotes/. + src := remoteRef + if !isFullRef(src) { + src = "refs/heads/" + src + } + refspec := fmt.Sprintf("+%s:refs/remotes/%s/%s", src, remote, localBranch) fetchCmd := exec.CommandContext(ctx, "git", "fetch", "--", remote, refspec) fetchCmd.Stdout = os.Stdout fetchCmd.Stderr = os.Stderr @@ -708,7 +737,7 @@ func gitCheckout(ctx context.Context, remote, remoteRef, localBranch string, det return fmt.Errorf("fetching %s/%s: %w", remote, remoteRef, err) } - ref := remote + "/" + remoteRef + ref := remote + "/" + localBranch if detach { cmd := exec.CommandContext(ctx, "git", "checkout", "--detach", ref) diff --git a/internal/cli/pr_checkout_test.go b/internal/cli/pr_checkout_test.go index f3b37ac..933a67f 100644 --- a/internal/cli/pr_checkout_test.go +++ b/internal/cli/pr_checkout_test.go @@ -188,6 +188,86 @@ func pushBranchToRemote(t *testing.T, repoDir, remoteName, branchName string) { } } +// pushToRemoteRef creates a commit and pushes it to an arbitrary ref on the +// remote (e.g. refs/pull/42/head), mimicking Gitea/Forgejo PRs whose head +// branch is gone and only the pull ref remains. +func pushToRemoteRef(t *testing.T, repoDir, remoteName, ref string) { + t.Helper() + + testFile := filepath.Join(repoDir, "pullref.txt") + if err := os.WriteFile(testFile, []byte("content for "+ref+"\n"), 0644); err != nil { + t.Fatalf("writing test file: %v", err) + } + + commands := [][]string{ + {"git", "checkout", "-b", "tmp-pushref"}, + {"git", "add", "."}, + {"git", "commit", "-m", "commit for " + ref}, + {"git", "push", remoteName, "HEAD:" + ref}, + {"git", "checkout", "-"}, + {"git", "branch", "-D", "tmp-pushref"}, + } + + for _, args := range commands { + cmd := exec.Command(args[0], args[1:]...) + cmd.Dir = repoDir + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git command %v failed: %v\n%s", args, err, out) + } + } +} + +// TestPRCheckoutPullRef covers Gitea/Forgejo PRs whose head.ref is a full +// refs/pull//head ref rather than a branch name. The ref must be fetched +// as-is (not under refs/heads/) and the local branch falls back to pr-. +func TestPRCheckoutPullRef(t *testing.T) { + if testing.Short() { + t.Skip("skipping git integration test in short mode") + } + + checkoutCmd, _, _ := rootCmd.Find([]string{"pr", "checkout"}) + if checkoutCmd != nil { + _ = checkoutCmd.Flags().Set("detach", "false") + _ = checkoutCmd.Flags().Set("force", "false") + _ = checkoutCmd.Flags().Set("branch", "") + _ = checkoutCmd.Flags().Set("remote-name", "") + } + + originDir := setupBareRepo(t) + workDir := setupTestRepo(t, originDir) + pushToRemoteRef(t, workDir, "origin", "refs/pull/42/head") + t.Chdir(workDir) + + pr := &forges.PullRequest{ + Number: 42, + Head: forges.PRBranch{Ref: "refs/pull/42/head", SHA: "abc123"}, + } + resolve.SetTestForge( + &mockForge{prService: &mockPRService{pr: pr}}, + "testowner", "testrepo", "github.com", + ) + t.Cleanup(resolve.ResetTestForge) + + var buf bytes.Buffer + rootCmd.SetOut(&buf) + rootCmd.SetErr(&buf) + rootCmd.SetArgs([]string{"pr", "checkout", "42"}) + + if err := rootCmd.Execute(); err != nil { + t.Fatalf("unexpected error: %v\noutput: %s", err, buf.String()) + } + + cmd := exec.Command("git", "branch", "--show-current") + cmd.Dir = workDir + out, err := cmd.Output() + if err != nil { + t.Fatalf("getting current branch: %v", err) + } + if got := strings.TrimSpace(string(out)); got != "pr-42" { + t.Errorf("branch: want %q, got %q", "pr-42", got) + } +} + func TestEnsureRemote(t *testing.T) { tests := []struct { name string diff --git a/types.go b/types.go index bc04c18..8e55e19 100644 --- a/types.go +++ b/types.go @@ -240,7 +240,7 @@ type ForkInfo struct { // PRBranch holds branch info including the repository it belongs to. // For same-repo PRs, Fork is nil. For fork PRs, Fork points to the source repo. type PRBranch struct { - Ref string `json:"ref"` // branch name + Ref string `json:"ref"` // branch name, or a full ref (e.g. refs/pull//head) for a branchless PR SHA string `json:"sha,omitempty"` // commit SHA Fork *ForkInfo `json:"fork,omitempty"` // nil if same repo as target }