Repository: cli/cli
Pull request #10859 (2025-04-24 10:27:53)

Fix pr create when branch name contains slashes
## Description

Fixes https://github.com/cli/cli/issues/10857

In v2.71.0 we made [some large changes to begin respecting git configuration when determining the head ref of a PR in `pr create`](https://github.com/cli/cli/pull/10513). Part of this work involved parsing remote tracking refs returned by `git rev-parse` and `git show-ref`. Unfortunately, I made a silly assumption that remotes and branches wouldn't contain slashes (extra silly because I use slashes in my branches).

This PR modifies `ParseRemoteTrackingRef` to accept slashes under the assumption that they belong to the branch name, as opposed to the remote name. So `refs/remotes/foo/bar/baz` will be parsed as `{ Remote: "foo", Branch: "bar/baz"}`.

Further work will need to be done to address slashes in remote names, but branch names are likely by far the majority case here, so let's get this fixed. Evidence that remote names are less of an issue come from the fact they've been broken for several months because the previous code actually made the same assumptions around assuming remotes were only one path component long when there was ambiguity. That is because `SplitN` works left to right, so `refs/remotes/foo/bar/baz` would become `[refs remotes foo bar/baz]`

https://github.com/cli/cli/blob/408e21ebdddf9cd14289e49135389a6e5125eff4/pkg/cmd/pr/create/create.go#L532-L548

This code has been in for 3 and a half months (2.65.0) without complaint. However, the code **before** that handled the ambiguity correctly by holding onto a struct containing the remote and branch name separately, and referring back to it later:
https://github.com/cli/cli/blob/44ee17760709bedf1cad7e452e61751489b81a33/pkg/cmd/pr/create/create.go#L513-L552

williammartin: All the acceptance tests pass:

```
--- PASS: TestPullRequests (0.00s)
    --- PASS: TestPullRequests/pr-create-no-local-repo (9.50s)
    --- PASS: TestPullRequests/pr-checkout-by-number (9.86s)
    --- PASS: TestPullRequests/pr-create-basic (10.23s)
    --- PASS: TestPullRequests/pr-comment-edit-last-without-comments-errors (10.25s)
    --- PASS: TestPullRequests/pr-create-from-issue-develop-base (15.36s)
    --- PASS: TestPullRequests/pr-create-respects-push-destination (19.36s)
    --- PASS: TestPullRequests/pr-view (10.20s)
    --- PASS: TestPullRequests/pr-view-status-respects-simple-pushdefault (11.30s)
    --- PASS: TestPullRequests/pr-create-remote-ref-with-branch-name-slash (21.53s)
    --- PASS: TestPullRequests/pr-create-guesses-remote-from-sha (21.90s)
    --- PASS: TestPullRequests/pr-create-guesses-remote-from-sha-with-remote-slash (22.02s)
    --- PASS: TestPullRequests/pr-merge-rebase-strategy (14.24s)
    --- PASS: TestPullRequests/pr-view-status-respects-push-destination (10.16s)
    --- PASS: TestPullRequests/pr-view-status-respects-remote-pushdefault (18.57s)
    --- PASS: TestPullRequests/pr-view-outside-repo (7.36s)
    --- PASS: TestPullRequests/pr-create-respects-branch-pushremote (31.10s)
    --- PASS: TestPullRequests/pr-comment-edit-last-without-comments-creates (8.76s)
    --- PASS: TestPullRequests/pr-comment-edit-last-with-comments (11.21s)
    --- PASS: TestPullRequests/pr-view-same-org-fork (15.52s)
    --- PASS: TestPullRequests/pr-view-status-respects-branch-pushremote (17.52s)
    --- PASS: TestPullRequests/pr-create-from-manual-merge-base (9.02s)
    --- PASS: TestPullRequests/pr-checkout (7.57s)
    --- PASS: TestPullRequests/pr-list (6.79s)
    --- PASS: TestPullRequests/pr-status-respects-cross-org (17.82s)
    --- PASS: TestPullRequests/pr-create-edit-with-project (18.50s)
    --- PASS: TestPullRequests/pr-create-guesses-remote-from-sha-with-branch-name-slash (16.64s)
    --- PASS: TestPullRequests/pr-comment-new (8.45s)
    --- PASS: TestPullRequests/pr-create-without-upstream-config (7.79s)
    --- PASS: TestPullRequests/pr-create-guesses-remote-from-sha-with-remote-and-branch-name-slashes (16.88s)
    --- PASS: TestPullRequests/pr-create-respects-simple-pushdefault (7.72s)
    --- PASS: TestPullRequests/pr-create-with-metadata (9.52s)
    --- PASS: TestPullRequests/pr-merge-merge-strategy (19.90s)
    --- PASS: TestPullRequests/pr-checkout-with-url-from-fork (18.85s)
    --- PASS: TestPullRequests/pr-create-respects-user-colon-branch-syntax (15.68s)
    --- PASS: TestPullRequests/pr-create-respects-remote-pushdefault (21.97s)
PASS
```

Patch:
diff --git a/git/client.go b/git/client.go
index fe2819cf0..5f547c99c 100644
--- a/git/client.go
+++ b/git/client.go
@@ -518,15 +518,56 @@ func (r RemoteTrackingRef) String() string {
 
 // ParseRemoteTrackingRef parses a string of the form "refs/remotes/<remote>/<branch>" into
 // a RemoteTrackingBranch struct. If the string does not match this format, an error is returned.
+//
+// For now, we assume that refnames are of the format "<remote>/<branch>", where
+// the remote is a single path component, and branch may have many path components e.g.
+// "origin/my/branch" is valid as: {Remote: "origin", Branch: "my/branch"}
+// but "my/origin/branch" would parse incorrectly as: {Remote: "my", Branch: "origin/branch"}
+// I don't believe there is a way to fix this without providing the list of remotes to this function.
+//
+// It becomes particularly confusing if you have something like:
+//
+// ```
+// [remote "foo"]
+// 	url = https://github.com/williammartin/test-repo.git
+// 	fetch = +refs/heads/*:refs/remotes/foo/*
+// [remote "foo/bar"]
+// 	url = https://github.com/williammartin/test-repo.git
+// 	fetch = +refs/heads/*:refs/remotes/foo/bar/*
+// [branch "bar/baz"]
+// 	remote = foo
+// 	merge = refs/heads/bar/baz
+// [branch "baz"]
+// 	remote = foo/bar
+// 	merge = refs/heads/baz
+// ```
+//
+// These @{push} refs would resolve identically:
+//
+// ```
+// ➜ git rev-parse --symbolic-full-name baz@{push}
+// refs/remotes/foo/bar/baz
+
+// ➜ git rev-parse --symbolic-full-name bar/baz@{push}
+// refs/remotes/foo/bar/baz
+// ```
+//
+// When using this ref, git assumes it means `remote: foo` `branch: bar/baz`.
 func ParseRemoteTrackingRef(s string) (RemoteTrackingRef, error) {
-	parts := strings.Split(s, "/")
-	if len(parts) != 4 || parts[0] != "refs" || parts[1] != "remotes" {
+	prefix := "refs/remotes/"
+	if !strings.HasPrefix(s, prefix) {
+		return RemoteTrackingRef{}, fmt.Errorf("remote tracking branch must have format refs/remotes/<remote>/<branch> but was: %s", s)
+	}
+
+	refName := strings.TrimPrefix(s, prefix)
+	refNameParts := strings.SplitN(refName, "/", 2)
+	if len(refNameParts) != 2 {
 		return RemoteTrackingRef{}, fmt.Errorf("remote tracking branch must have format refs/remotes/<remote>/<branch> but was: %s", s)
 	}
 
 	return RemoteTrackingRef{
-		Remote: parts[2],
-		Branch: parts[3],
+		Remote: refNameParts[0],
+		Branch: refNameParts[1],
 	}, nil
 }
 


Test patch:
diff --git a/acceptance/testdata/pr/pr-create-guesses-remote-from-sha-with-branch-name-slash.txtar b/acceptance/testdata/pr/pr-create-guesses-remote-from-sha-with-branch-name-slash.txtar
new file mode 100644
index 000000000..542579b0a
--- /dev/null
+++ b/acceptance/testdata/pr/pr-create-guesses-remote-from-sha-with-branch-name-slash.txtar
@@ -0,0 +1,50 @@
+skip 'it creates a fork owned by the user running the test'
+
+env REPO=${SCRIPT_NAME}-${RANDOM_STRING}
+env FORK=${REPO}-fork
+
+# Use gh as a credential helper
+exec gh auth setup-git
+
+# Get the current username for the fork owner
+exec gh api user --jq .login
+stdout2env USER
+
+# Create a repository with a file so it has a default branch
+exec gh repo create ${ORG}/${REPO} --add-readme --private
+
+# Defer repo cleanup
+defer gh repo delete --yes ${ORG}/${REPO}
+
+# Create a user fork of repository. This will be owned by USER.
+exec gh repo fork ${ORG}/${REPO} --fork-name ${FORK}
+sleep 5
+
+# Defer repo cleanup of fork
+defer gh repo delete --yes ${USER}/${FORK}
+
+# Retrieve fork repository information
+exec gh repo view ${USER}/${FORK} --json id --jq '.id'
+stdout2env FORK_ID
+
+exec gh repo clone ${USER}/${FORK}
+cd ${FORK}
+
+# Prepare a branch to commit
+exec git checkout -b feature/branch
+exec git commit --allow-empty -m 'Upstream Commit'
+# Push without setting an upstream (-u or config)
+exec git push upstream feature/branch
+
+# Prepare an additional commit
+exec git commit --allow-empty -m 'Fork Commit'
+# Push without setting an upstream (-u or config)
+exec git push origin feature/branch
+
+# Create the PR
+exec gh pr create --title 'Feature Title' --body 'Feature Body'
+stdout https://${GH_HOST}/${ORG}/${REPO}/pull/1
+
+# Check the PR is indeed created
+exec gh pr view ${USER}:feature/branch --json headRefName,headRepository,baseRefName,isCrossRepository
+stdout {"baseRefName":"main","headRefName":"feature/branch","headRepository":{"id":"${FORK_ID}","name":"${FORK}"},"isCrossRepository":true}
diff --git a/acceptance/testdata/pr/pr-create-guesses-remote-from-sha.txtar b/acceptance/testdata/pr/pr-create-guesses-remote-from-sha.txtar
index 52579b501..e263b0351 100644
--- a/acceptance/testdata/pr/pr-create-guesses-remote-from-sha.txtar
+++ b/acceptance/testdata/pr/pr-create-guesses-remote-from-sha.txtar
@@ -1,3 +1,5 @@
+skip 'it creates a fork owned by the user running the test'
+
 env REPO=${SCRIPT_NAME}-${RANDOM_STRING}
 env FORK=${REPO}-fork
 
diff --git a/acceptance/testdata/pr/pr-create-remote-ref-with-branch-name-slash.txtar b/acceptance/testdata/pr/pr-create-remote-ref-with-branch-name-slash.txtar
new file mode 100644
index 000000000..395fce86a
--- /dev/null
+++ b/acceptance/testdata/pr/pr-create-remote-ref-with-branch-name-slash.txtar
@@ -0,0 +1,46 @@
+skip 'it creates a fork owned by the user running the test'
+
+# Setup environment variables used for testscript
+env REPO=${SCRIPT_NAME}-${RANDOM_STRING}
+env FORK=${REPO}-fork
+
+# Use gh as a credential helper
+exec gh auth setup-git
+
+# Get the current username for the fork owner
+exec gh api user --jq .login
+stdout2env USER
+
+# Create a repository to act as upstream with a file so it has a default branch
+exec gh repo create ${ORG}/${REPO} --add-readme --private
+
+# Defer repo cleanup of upstream
+defer gh repo delete --yes ${ORG}/${REPO}
+
+# Create a user fork of repository. This will be owned by USER.
+exec gh repo fork ${ORG}/${REPO} --fork-name ${FORK}
+sleep 5
+
+# Defer repo cleanup of fork
+defer gh repo delete --yes ${USER}/${FORK}
+
+# Retrieve fork repository information
+exec gh repo view ${USER}/${FORK} --json id --jq '.id'
+stdout2env FORK_ID
+
+# Clone the repo
+exec gh repo clone ${USER}/${FORK}
+cd ${FORK}
+
+# Prepare a branch where changes are pulled from the upstream default branch but pushed to fork
+exec git checkout -b feature/branch
+exec git commit --allow-empty -m 'Empty Commit'
+exec git push -u origin feature/branch
+
+# Create the PR spanning upstream and fork repositories
+exec gh pr create --title 'Feature Title' --body 'Feature Body'
+stdout https://${GH_HOST}/${ORG}/${REPO}/pull/1
+
+# Assert that the PR was created with the correct head repository and refs
+exec gh pr view --json headRefName,headRepository,baseRefName,isCrossRepository
+stdout {"baseRefName":"main","headRefName":"feature/branch","headRepository":{"id":"${FORK_ID}","name":"${FORK}"},"isCrossRepository":true}
diff --git a/git/client_test.go b/git/client_test.go
index 3d7560228..f59b26077 100644
--- a/git/client_test.go
+++ b/git/client_test.go
@@ -1151,13 +1151,38 @@ func TestRemoteTrackingRef(t *testing.T) {
 			wantError             error
 		}{
 			{
-				name:              "valid remote tracking ref",
+				name:              "valid remote tracking ref without slash in branch name",
 				remoteTrackingRef: "refs/remotes/origin/branchName",
 				wantRemoteTrackingRef: RemoteTrackingRef{
 					Remote: "origin",
 					Branch: "branchName",
 				},
 			},
+			{
+				name:              "valid remote tracking ref with slash in branch name",
+				remoteTrackingRef: "refs/remotes/origin/branch/name",
+				wantRemoteTrackingRef: RemoteTrackingRef{
+					Remote: "origin",
+					Branch: "branch/name",
+				},
+			},
+			// TODO: Uncomment when we support slashes in remote names
+			// {
+			// 	name: "valid remote tracking ref with slash in remote name",
+			// 	remoteTrackingRef: "refs/remotes/my/origin/branchName",
+			// 	wantRemoteTrackingRef: RemoteTrackingRef{
+			// 		Remote: "my/origin",
+			// 		Branch: "branchName",
+			// 	},
+			// },
+			// {
+			// 	name: 			"valid remote tracking ref with slash in remote name and branch name",
+			// 	remoteTrackingRef: "refs/remotes/my/origin/branch/name",
+			// 	wantRemoteTrackingRef: RemoteTrackingRef{
+			// 		Remote: "my/origin",
+			// 		Branch: "branch/name",
+			// 	},
+			// },
 			{
 				name:                  "incorrect parts",
 				remoteTrackingRef:     "refs/remotes/origin",