From a542011cdce8fb579089a0e6a8284526a19d21d3 Mon Sep 17 00:00:00 2001
From: ChandranshuRao14 <chandranshu.rao@gmail.com>
Date: Tue, 24 Dec 2024 12:59:42 -0600
Subject: [PATCH 01/25] Feat: Allow setting security_and_analysis settings in
 gh repo edit

---
 pkg/cmd/repo/edit/edit.go      | 92 ++++++++++++++++++++++++++++------
 pkg/cmd/repo/edit/edit_test.go | 30 +++++++++++
 2 files changed, 106 insertions(+), 16 deletions(-)

diff --git a/pkg/cmd/repo/edit/edit.go b/pkg/cmd/repo/edit/edit.go
index 29a149cf9..80e460186 100644
--- a/pkg/cmd/repo/edit/edit.go
+++ b/pkg/cmd/repo/edit/edit.go
@@ -66,22 +66,37 @@ type EditOptions struct {
 }
 
 type EditRepositoryInput struct {
-	AllowForking        *bool   `json:"allow_forking,omitempty"`
-	AllowUpdateBranch   *bool   `json:"allow_update_branch,omitempty"`
-	DefaultBranch       *string `json:"default_branch,omitempty"`
-	DeleteBranchOnMerge *bool   `json:"delete_branch_on_merge,omitempty"`
-	Description         *string `json:"description,omitempty"`
-	EnableAutoMerge     *bool   `json:"allow_auto_merge,omitempty"`
-	EnableIssues        *bool   `json:"has_issues,omitempty"`
-	EnableMergeCommit   *bool   `json:"allow_merge_commit,omitempty"`
-	EnableProjects      *bool   `json:"has_projects,omitempty"`
-	EnableDiscussions   *bool   `json:"has_discussions,omitempty"`
-	EnableRebaseMerge   *bool   `json:"allow_rebase_merge,omitempty"`
-	EnableSquashMerge   *bool   `json:"allow_squash_merge,omitempty"`
-	EnableWiki          *bool   `json:"has_wiki,omitempty"`
-	Homepage            *string `json:"homepage,omitempty"`
-	IsTemplate          *bool   `json:"is_template,omitempty"`
-	Visibility          *string `json:"visibility,omitempty"`
+	enableAdvancedSecurity             *bool
+	enableSecretScanning               *bool
+	enableSecretScanningPushProtection *bool
+
+	AllowForking        *bool                     `json:"allow_forking,omitempty"`
+	AllowUpdateBranch   *bool                     `json:"allow_update_branch,omitempty"`
+	DefaultBranch       *string                   `json:"default_branch,omitempty"`
+	DeleteBranchOnMerge *bool                     `json:"delete_branch_on_merge,omitempty"`
+	Description         *string                   `json:"description,omitempty"`
+	EnableAutoMerge     *bool                     `json:"allow_auto_merge,omitempty"`
+	EnableIssues        *bool                     `json:"has_issues,omitempty"`
+	EnableMergeCommit   *bool                     `json:"allow_merge_commit,omitempty"`
+	EnableProjects      *bool                     `json:"has_projects,omitempty"`
+	EnableDiscussions   *bool                     `json:"has_discussions,omitempty"`
+	EnableRebaseMerge   *bool                     `json:"allow_rebase_merge,omitempty"`
+	EnableSquashMerge   *bool                     `json:"allow_squash_merge,omitempty"`
+	EnableWiki          *bool                     `json:"has_wiki,omitempty"`
+	Homepage            *string                   `json:"homepage,omitempty"`
+	IsTemplate          *bool                     `json:"is_template,omitempty"`
+	SecurityAndAnalysis *SecurityAndAnalysisInput `json:"security_and_analysis,omitempty"`
+	Visibility          *string                   `json:"visibility,omitempty"`
+}
+
+type SecurityAndAnalysisInput struct {
+	EnableAdvancedSecurity             *SecurityAndAnalysisStatus `json:"advanced_security,omitempty"`
+	EnableSecretScanning               *SecurityAndAnalysisStatus `json:"secret_scanning,omitempty"`
+	EnableSecretScanningPushProtection *SecurityAndAnalysisStatus `json:"secret_scanning_push_protection,omitempty"`
+}
+
+type SecurityAndAnalysisStatus struct {
+	Status *string `json:"status,omitempty"`
 }
 
 func NewCmdEdit(f *cmdutil.Factory, runF func(options *EditOptions) error) *cobra.Command {
@@ -177,6 +192,9 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(options *EditOptions) error) *cobr
 	cmdutil.NilBoolFlag(cmd, &opts.Edits.EnableSquashMerge, "enable-squash-merge", "", "Enable merging pull requests via squashed commit")
 	cmdutil.NilBoolFlag(cmd, &opts.Edits.EnableRebaseMerge, "enable-rebase-merge", "", "Enable merging pull requests via rebase")
 	cmdutil.NilBoolFlag(cmd, &opts.Edits.EnableAutoMerge, "enable-auto-merge", "", "Enable auto-merge functionality")
+	cmdutil.NilBoolFlag(cmd, &opts.Edits.enableAdvancedSecurity, "enable-advanced-security", "", "Enable advanced security in the repository")
+	cmdutil.NilBoolFlag(cmd, &opts.Edits.enableSecretScanning, "enable-secret-scanning", "", "Enable secret scanning in the repository")
+	cmdutil.NilBoolFlag(cmd, &opts.Edits.enableSecretScanningPushProtection, "enable-secret-scanning-push-protection", "", "Enable secret scanning push protection in the repository. Secret scanning must be enabled first")
 	cmdutil.NilBoolFlag(cmd, &opts.Edits.DeleteBranchOnMerge, "delete-branch-on-merge", "", "Delete head branch when pull requests are merged")
 	cmdutil.NilBoolFlag(cmd, &opts.Edits.AllowForking, "allow-forking", "", "Allow forking of an organization repository")
 	cmdutil.NilBoolFlag(cmd, &opts.Edits.AllowUpdateBranch, "allow-update-branch", "", "Allow a pull request head branch that is behind its base branch to be updated")
@@ -240,6 +258,34 @@ func editRun(ctx context.Context, opts *EditOptions) error {
 		}
 	}
 
+	if hasSecurityEdits(opts.Edits) {
+		apiClient := api.NewClientFromHTTP(opts.HTTPClient)
+		repo, err := api.FetchRepository(apiClient, opts.Repository, []string{"viewerCanAdminister"})
+		if err != nil {
+			return err
+		}
+		if !repo.ViewerCanAdminister {
+			return fmt.Errorf("you do not have sufficient permissions to edit repository security and analysis features")
+		}
+
+		opts.Edits.SecurityAndAnalysis = &SecurityAndAnalysisInput{}
+		if opts.Edits.enableAdvancedSecurity != nil {
+			opts.Edits.SecurityAndAnalysis.EnableAdvancedSecurity = &SecurityAndAnalysisStatus{
+				Status: boolToStatus(*opts.Edits.enableAdvancedSecurity),
+			}
+		}
+		if opts.Edits.enableSecretScanning != nil {
+			opts.Edits.SecurityAndAnalysis.EnableSecretScanning = &SecurityAndAnalysisStatus{
+				Status: boolToStatus(*opts.Edits.enableSecretScanning),
+			}
+		}
+		if opts.Edits.enableSecretScanningPushProtection != nil {
+			opts.Edits.SecurityAndAnalysis.EnableSecretScanningPushProtection = &SecurityAndAnalysisStatus{
+				Status: boolToStatus(*opts.Edits.enableSecretScanningPushProtection),
+			}
+		}
+	}
+
 	apiPath := fmt.Sprintf("repos/%s/%s", repo.RepoOwner(), repo.RepoName())
 
 	body := &bytes.Buffer{}
@@ -560,3 +606,17 @@ func isIncluded(value string, opts []string) bool {
 	}
 	return false
 }
+
+func boolToStatus(status bool) *string {
+	var result string
+	if status {
+		result = "enabled"
+	} else {
+		result = "disabled"
+	}
+	return &result
+}
+
+func hasSecurityEdits(edits EditRepositoryInput) bool {
+	return edits.enableAdvancedSecurity != nil || edits.enableSecretScanning != nil || edits.enableSecretScanningPushProtection != nil
+}
diff --git a/pkg/cmd/repo/edit/edit_test.go b/pkg/cmd/repo/edit/edit_test.go
index 217c1dce4..93d44788d 100644
--- a/pkg/cmd/repo/edit/edit_test.go
+++ b/pkg/cmd/repo/edit/edit_test.go
@@ -201,6 +201,36 @@ func Test_editRun(t *testing.T) {
 					}))
 			},
 		},
+		{
+			name: "enable/disable security and analysis settings",
+			opts: EditOptions{
+				Repository: ghrepo.NewWithHost("OWNER", "REPO", "github.com"),
+				Edits: EditRepositoryInput{
+					SecurityAndAnalysis: &SecurityAndAnalysisInput{
+						EnableAdvancedSecurity: &SecurityAndAnalysisStatus{
+							Status: sp("enabled"),
+						},
+						EnableSecretScanning: &SecurityAndAnalysisStatus{
+							Status: sp("enabled"),
+						},
+						EnableSecretScanningPushProtection: &SecurityAndAnalysisStatus{
+							Status: sp("disabled"),
+						},
+					},
+				},
+			},
+			httpStubs: func(t *testing.T, r *httpmock.Registry) {
+				r.Register(
+					httpmock.REST("PATCH", "repos/OWNER/REPO"),
+					httpmock.RESTPayload(200, `{}`, func(payload map[string]interface{}) {
+						assert.Equal(t, 1, len(payload))
+						securityAndAnalysis := payload["security_and_analysis"].(map[string]interface{})
+						assert.Equal(t, "enabled", securityAndAnalysis["advanced_security"].(map[string]interface{})["status"])
+						assert.Equal(t, "enabled", securityAndAnalysis["secret_scanning"].(map[string]interface{})["status"])
+						assert.Equal(t, "disabled", securityAndAnalysis["secret_scanning_push_protection"].(map[string]interface{})["status"])
+					}))
+			},
+		},
 	}
 
 	for _, tt := range tests {

From 29371e949126db53cc8d93132d23812a06fc72fd Mon Sep 17 00:00:00 2001
From: ChandranshuRao14 <chandranshu.rao@gmail.com>
Date: Thu, 26 Dec 2024 22:52:32 -0500
Subject: [PATCH 02/25] Extract logic into helper function

---
 pkg/cmd/repo/edit/edit.go      | 82 ++++++++++++++++---------------
 pkg/cmd/repo/edit/edit_test.go | 89 ++++++++++++++++++++++++++++++++++
 2 files changed, 133 insertions(+), 38 deletions(-)

diff --git a/pkg/cmd/repo/edit/edit.go b/pkg/cmd/repo/edit/edit.go
index 80e460186..8efcea11d 100644
--- a/pkg/cmd/repo/edit/edit.go
+++ b/pkg/cmd/repo/edit/edit.go
@@ -89,16 +89,6 @@ type EditRepositoryInput struct {
 	Visibility          *string                   `json:"visibility,omitempty"`
 }
 
-type SecurityAndAnalysisInput struct {
-	EnableAdvancedSecurity             *SecurityAndAnalysisStatus `json:"advanced_security,omitempty"`
-	EnableSecretScanning               *SecurityAndAnalysisStatus `json:"secret_scanning,omitempty"`
-	EnableSecretScanningPushProtection *SecurityAndAnalysisStatus `json:"secret_scanning_push_protection,omitempty"`
-}
-
-type SecurityAndAnalysisStatus struct {
-	Status *string `json:"status,omitempty"`
-}
-
 func NewCmdEdit(f *cmdutil.Factory, runF func(options *EditOptions) error) *cobra.Command {
 	opts := &EditOptions{
 		IO:       f.IOStreams,
@@ -172,6 +162,18 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(options *EditOptions) error) *cobr
 				return cmdutil.FlagErrorf("use of --visibility flag requires --accept-visibility-change-consequences flag")
 			}
 
+			if hasSecurityEdits(opts.Edits) {
+				apiClient := api.NewClientFromHTTP(opts.HTTPClient)
+				repo, err := api.FetchRepository(apiClient, opts.Repository, []string{"viewerCanAdminister"})
+				if err != nil {
+					return err
+				}
+				if !repo.ViewerCanAdminister {
+					return fmt.Errorf("you do not have sufficient permissions to edit repository security and analysis features")
+				}
+				opts.Edits.SecurityAndAnalysis = transformSecurityAndAnalysisOpts(opts)
+			}
+
 			if runF != nil {
 				return runF(opts)
 			}
@@ -258,34 +260,6 @@ func editRun(ctx context.Context, opts *EditOptions) error {
 		}
 	}
 
-	if hasSecurityEdits(opts.Edits) {
-		apiClient := api.NewClientFromHTTP(opts.HTTPClient)
-		repo, err := api.FetchRepository(apiClient, opts.Repository, []string{"viewerCanAdminister"})
-		if err != nil {
-			return err
-		}
-		if !repo.ViewerCanAdminister {
-			return fmt.Errorf("you do not have sufficient permissions to edit repository security and analysis features")
-		}
-
-		opts.Edits.SecurityAndAnalysis = &SecurityAndAnalysisInput{}
-		if opts.Edits.enableAdvancedSecurity != nil {
-			opts.Edits.SecurityAndAnalysis.EnableAdvancedSecurity = &SecurityAndAnalysisStatus{
-				Status: boolToStatus(*opts.Edits.enableAdvancedSecurity),
-			}
-		}
-		if opts.Edits.enableSecretScanning != nil {
-			opts.Edits.SecurityAndAnalysis.EnableSecretScanning = &SecurityAndAnalysisStatus{
-				Status: boolToStatus(*opts.Edits.enableSecretScanning),
-			}
-		}
-		if opts.Edits.enableSecretScanningPushProtection != nil {
-			opts.Edits.SecurityAndAnalysis.EnableSecretScanningPushProtection = &SecurityAndAnalysisStatus{
-				Status: boolToStatus(*opts.Edits.enableSecretScanningPushProtection),
-			}
-		}
-	}
-
 	apiPath := fmt.Sprintf("repos/%s/%s", repo.RepoOwner(), repo.RepoName())
 
 	body := &bytes.Buffer{}
@@ -620,3 +594,35 @@ func boolToStatus(status bool) *string {
 func hasSecurityEdits(edits EditRepositoryInput) bool {
 	return edits.enableAdvancedSecurity != nil || edits.enableSecretScanning != nil || edits.enableSecretScanningPushProtection != nil
 }
+
+type SecurityAndAnalysisInput struct {
+	EnableAdvancedSecurity             *SecurityAndAnalysisStatus `json:"advanced_security,omitempty"`
+	EnableSecretScanning               *SecurityAndAnalysisStatus `json:"secret_scanning,omitempty"`
+	EnableSecretScanningPushProtection *SecurityAndAnalysisStatus `json:"secret_scanning_push_protection,omitempty"`
+}
+
+type SecurityAndAnalysisStatus struct {
+	Status *string `json:"status,omitempty"`
+}
+
+// Transform security and analysis parameters to properly serialize EditRepositoryInput
+// See API Docs: https://docs.github.com/en/rest/repos/repos?apiVersion=2022-11-28#update-a-repository
+func transformSecurityAndAnalysisOpts(opts *EditOptions) *SecurityAndAnalysisInput {
+	securityOptions := &SecurityAndAnalysisInput{}
+	if opts.Edits.enableAdvancedSecurity != nil {
+		securityOptions.EnableAdvancedSecurity = &SecurityAndAnalysisStatus{
+			Status: boolToStatus(*opts.Edits.enableAdvancedSecurity),
+		}
+	}
+	if opts.Edits.enableSecretScanning != nil {
+		securityOptions.EnableSecretScanning = &SecurityAndAnalysisStatus{
+			Status: boolToStatus(*opts.Edits.enableSecretScanning),
+		}
+	}
+	if opts.Edits.enableSecretScanningPushProtection != nil {
+		securityOptions.EnableSecretScanningPushProtection = &SecurityAndAnalysisStatus{
+			Status: boolToStatus(*opts.Edits.enableSecretScanningPushProtection),
+		}
+	}
+	return securityOptions
+}
diff --git a/pkg/cmd/repo/edit/edit_test.go b/pkg/cmd/repo/edit/edit_test.go
index 93d44788d..93b256465 100644
--- a/pkg/cmd/repo/edit/edit_test.go
+++ b/pkg/cmd/repo/edit/edit_test.go
@@ -700,6 +700,95 @@ func Test_editRun_interactive(t *testing.T) {
 	}
 }
 
+func Test_transformSecurityAndAnalysisOpts(t *testing.T) {
+	tests := []struct {
+		name string
+		opts EditOptions
+		want *SecurityAndAnalysisInput
+	}{
+		{
+			name: "Enable all security and analysis settings",
+			opts: EditOptions{
+				Edits: EditRepositoryInput{
+					enableAdvancedSecurity:             bp(true),
+					enableSecretScanning:               bp(true),
+					enableSecretScanningPushProtection: bp(true),
+				},
+			},
+			want: &SecurityAndAnalysisInput{
+				EnableAdvancedSecurity: &SecurityAndAnalysisStatus{
+					Status: sp("enabled"),
+				},
+				EnableSecretScanning: &SecurityAndAnalysisStatus{
+					Status: sp("enabled"),
+				},
+				EnableSecretScanningPushProtection: &SecurityAndAnalysisStatus{
+					Status: sp("enabled"),
+				},
+			},
+		},
+		{
+			name: "Disable all security and analysis settings",
+			opts: EditOptions{
+				Edits: EditRepositoryInput{
+					enableAdvancedSecurity:             bp(false),
+					enableSecretScanning:               bp(false),
+					enableSecretScanningPushProtection: bp(false),
+				},
+			},
+			want: &SecurityAndAnalysisInput{
+				EnableAdvancedSecurity: &SecurityAndAnalysisStatus{
+					Status: sp("disabled"),
+				},
+				EnableSecretScanning: &SecurityAndAnalysisStatus{
+					Status: sp("disabled"),
+				},
+				EnableSecretScanningPushProtection: &SecurityAndAnalysisStatus{
+					Status: sp("disabled"),
+				},
+			},
+		},
+		{
+			name: "Enable only advanced security",
+			opts: EditOptions{
+				Edits: EditRepositoryInput{
+					enableAdvancedSecurity: bp(true),
+				},
+			},
+			want: &SecurityAndAnalysisInput{
+				EnableAdvancedSecurity: &SecurityAndAnalysisStatus{
+					Status: sp("enabled"),
+				},
+				EnableSecretScanning:               nil,
+				EnableSecretScanningPushProtection: nil,
+			},
+		},
+		{
+			name: "Disable only secret scanning",
+			opts: EditOptions{
+				Edits: EditRepositoryInput{
+					enableSecretScanning: bp(false),
+				},
+			},
+			want: &SecurityAndAnalysisInput{
+				EnableAdvancedSecurity: nil,
+				EnableSecretScanning: &SecurityAndAnalysisStatus{
+					Status: sp("disabled"),
+				},
+				EnableSecretScanningPushProtection: nil,
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			opts := &tt.opts
+			transformed := transformSecurityAndAnalysisOpts(opts)
+			assert.Equal(t, tt.want, transformed)
+		})
+	}
+}
+
 func sp(v string) *string {
 	return &v
 }

From 079719f9230cd7ccdc26c23375016272a5d13ac3 Mon Sep 17 00:00:00 2001
From: ChandranshuRao14 <chandranshu.rao@gmail.com>
Date: Tue, 31 Dec 2024 00:47:05 -0500
Subject: [PATCH 03/25] Move api call to editRun

---
 pkg/cmd/repo/edit/edit.go | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/pkg/cmd/repo/edit/edit.go b/pkg/cmd/repo/edit/edit.go
index 8efcea11d..cacc9420d 100644
--- a/pkg/cmd/repo/edit/edit.go
+++ b/pkg/cmd/repo/edit/edit.go
@@ -163,14 +163,6 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(options *EditOptions) error) *cobr
 			}
 
 			if hasSecurityEdits(opts.Edits) {
-				apiClient := api.NewClientFromHTTP(opts.HTTPClient)
-				repo, err := api.FetchRepository(apiClient, opts.Repository, []string{"viewerCanAdminister"})
-				if err != nil {
-					return err
-				}
-				if !repo.ViewerCanAdminister {
-					return fmt.Errorf("you do not have sufficient permissions to edit repository security and analysis features")
-				}
 				opts.Edits.SecurityAndAnalysis = transformSecurityAndAnalysisOpts(opts)
 			}
 
@@ -260,6 +252,17 @@ func editRun(ctx context.Context, opts *EditOptions) error {
 		}
 	}
 
+	if hasSecurityEdits(opts.Edits) {
+		apiClient := api.NewClientFromHTTP(opts.HTTPClient)
+		repo, err := api.FetchRepository(apiClient, opts.Repository, []string{"viewerCanAdminister"})
+		if err != nil {
+			return err
+		}
+		if !repo.ViewerCanAdminister {
+			return fmt.Errorf("you do not have sufficient permissions to edit repository security and analysis features")
+		}
+	}
+
 	apiPath := fmt.Sprintf("repos/%s/%s", repo.RepoOwner(), repo.RepoName())
 
 	body := &bytes.Buffer{}

From 8560c24f3fbbd828a07c4fef52ddd6e3bc5abf98 Mon Sep 17 00:00:00 2001
From: Aryan Bhosale <36108149+aryanbhosale@users.noreply.github.com>
Date: Wed, 1 Jan 2025 11:56:18 +0530
Subject: [PATCH 04/25] fix(repo fork): add non-TTY output when fork is newly
 created

---
 pkg/cmd/repo/fork/fork.go | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/pkg/cmd/repo/fork/fork.go b/pkg/cmd/repo/fork/fork.go
index 73e269500..971adcb41 100644
--- a/pkg/cmd/repo/fork/fork.go
+++ b/pkg/cmd/repo/fork/fork.go
@@ -221,6 +221,8 @@ func forkRun(opts *ForkOptions) error {
 	} else {
 		if connectedToTerminal {
 			fmt.Fprintf(stderr, "%s Created fork %s\n", cs.SuccessIconWithColor(cs.Green), cs.Bold(ghrepo.FullName(forkedRepo)))
+		} else {
+			fmt.Fprintf(stderr, "Created fork %s\n", ghrepo.FullName(forkedRepo))
 		}
 	}
 

From 8dbbceaaafbc31d1af9e8c9be1cca58d1e28fa9b Mon Sep 17 00:00:00 2001
From: Aryan Bhosale <36108149+aryanbhosale@users.noreply.github.com>
Date: Fri, 3 Jan 2025 10:54:30 +0530
Subject: [PATCH 05/25] print repo url to stdout

---
 pkg/cmd/repo/fork/fork.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pkg/cmd/repo/fork/fork.go b/pkg/cmd/repo/fork/fork.go
index 971adcb41..3d62a73cc 100644
--- a/pkg/cmd/repo/fork/fork.go
+++ b/pkg/cmd/repo/fork/fork.go
@@ -222,7 +222,7 @@ func forkRun(opts *ForkOptions) error {
 		if connectedToTerminal {
 			fmt.Fprintf(stderr, "%s Created fork %s\n", cs.SuccessIconWithColor(cs.Green), cs.Bold(ghrepo.FullName(forkedRepo)))
 		} else {
-			fmt.Fprintf(stderr, "Created fork %s\n", ghrepo.FullName(forkedRepo))
+			fmt.Fprintln(opts.IO.Out, ghrepo.GenerateRepoURL(forkedRepo, ""))
 		}
 	}
 

From 576fa8a3bc97169f24fd5264dc4f4114027308c6 Mon Sep 17 00:00:00 2001
From: Tyler McGoffin <jtmcg@github.com>
Date: Thu, 2 Jan 2025 22:29:45 -0800
Subject: [PATCH 06/25] Add test for permissions check for security and
 analysis edits (#1)

---
 pkg/cmd/repo/edit/edit.go      |  2 +-
 pkg/cmd/repo/edit/edit_test.go | 29 +++++++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/pkg/cmd/repo/edit/edit.go b/pkg/cmd/repo/edit/edit.go
index cacc9420d..daa700cfb 100644
--- a/pkg/cmd/repo/edit/edit.go
+++ b/pkg/cmd/repo/edit/edit.go
@@ -252,7 +252,7 @@ func editRun(ctx context.Context, opts *EditOptions) error {
 		}
 	}
 
-	if hasSecurityEdits(opts.Edits) {
+	if opts.Edits.SecurityAndAnalysis != nil {
 		apiClient := api.NewClientFromHTTP(opts.HTTPClient)
 		repo, err := api.FetchRepository(apiClient, opts.Repository, []string{"viewerCanAdminister"})
 		if err != nil {
diff --git a/pkg/cmd/repo/edit/edit_test.go b/pkg/cmd/repo/edit/edit_test.go
index 93b256465..868e300fa 100644
--- a/pkg/cmd/repo/edit/edit_test.go
+++ b/pkg/cmd/repo/edit/edit_test.go
@@ -220,6 +220,10 @@ func Test_editRun(t *testing.T) {
 				},
 			},
 			httpStubs: func(t *testing.T, r *httpmock.Registry) {
+				r.Register(
+					httpmock.GraphQL(`query RepositoryInfo\b`),
+					httpmock.StringResponse(`{"data": { "repository": { "viewerCanAdminister": true } } }`))
+
 				r.Register(
 					httpmock.REST("PATCH", "repos/OWNER/REPO"),
 					httpmock.RESTPayload(200, `{}`, func(payload map[string]interface{}) {
@@ -231,6 +235,31 @@ func Test_editRun(t *testing.T) {
 					}))
 			},
 		},
+		{
+			name: "does not have sufficient permissions for security edits",
+			opts: EditOptions{
+				Repository: ghrepo.NewWithHost("OWNER", "REPO", "github.com"),
+				Edits: EditRepositoryInput{
+					SecurityAndAnalysis: &SecurityAndAnalysisInput{
+						EnableAdvancedSecurity: &SecurityAndAnalysisStatus{
+							Status: sp("enabled"),
+						},
+						EnableSecretScanning: &SecurityAndAnalysisStatus{
+							Status: sp("enabled"),
+						},
+						EnableSecretScanningPushProtection: &SecurityAndAnalysisStatus{
+							Status: sp("disabled"),
+						},
+					},
+				},
+			},
+			httpStubs: func(t *testing.T, r *httpmock.Registry) {
+				r.Register(
+					httpmock.GraphQL(`query RepositoryInfo\b`),
+					httpmock.StringResponse(`{"data": { "repository": { "viewerCanAdminister": false } } }`))
+			},
+			wantsErr: "you do not have sufficient permissions to edit repository security and analysis features",
+		},
 	}
 
 	for _, tt := range tests {

From 7a1052ca339a2475fddd08c22cca1eacd8707251 Mon Sep 17 00:00:00 2001
From: Caleb Brose <5447118+cmbrose@users.noreply.github.com>
Date: Fri, 3 Jan 2025 20:35:48 +0000
Subject: [PATCH 07/25] Set LocalBranch even if the git config fails

---
 .../pr-create-without-upstream-config.txtar   | 27 +++++++++++++++++++
 git/client.go                                 |  3 ++-
 2 files changed, 29 insertions(+), 1 deletion(-)
 create mode 100644 acceptance/testdata/pr/pr-create-without-upstream-config.txtar

diff --git a/acceptance/testdata/pr/pr-create-without-upstream-config.txtar b/acceptance/testdata/pr/pr-create-without-upstream-config.txtar
new file mode 100644
index 000000000..00f3535a7
--- /dev/null
+++ b/acceptance/testdata/pr/pr-create-without-upstream-config.txtar
@@ -0,0 +1,27 @@
+# This test is the same as pr-create-basic, except that the git push doesn't include the -u argument
+# This causes a git config read to fail during gh pr create, but it should not be fatal
+
+# Use gh as a credential helper
+exec gh auth setup-git
+
+# Create a repository with a file so it has a default branch
+exec gh repo create $ORG/$SCRIPT_NAME-$RANDOM_STRING --add-readme --private
+
+# Defer repo cleanup
+defer gh repo delete --yes $ORG/$SCRIPT_NAME-$RANDOM_STRING
+
+# Clone the repo
+exec gh repo clone $ORG/$SCRIPT_NAME-$RANDOM_STRING
+
+# Prepare a branch to PR
+cd $SCRIPT_NAME-$RANDOM_STRING
+exec git checkout -b feature-branch
+exec git commit --allow-empty -m 'Empty Commit'
+exec git push origin feature-branch
+
+# Create the PR
+exec gh pr create --title 'Feature Title' --body 'Feature Body'
+
+# Check the PR is indeed created
+exec gh pr view
+stdout 'Feature Title'
diff --git a/git/client.go b/git/client.go
index 35b9cf16c..774ce5e76 100644
--- a/git/client.go
+++ b/git/client.go
@@ -378,6 +378,8 @@ func (c *Client) lookupCommit(ctx context.Context, sha, format string) ([]byte,
 
 // ReadBranchConfig parses the `branch.BRANCH.(remote|merge|gh-merge-base)` part of git config.
 func (c *Client) ReadBranchConfig(ctx context.Context, branch string) (cfg BranchConfig) {
+	cfg.LocalName = branch
+
 	prefix := regexp.QuoteMeta(fmt.Sprintf("branch.%s.", branch))
 	args := []string{"config", "--get-regexp", fmt.Sprintf("^%s(remote|merge|%s)$", prefix, MergeBaseConfig)}
 	cmd, err := c.Command(ctx, args...)
@@ -389,7 +391,6 @@ func (c *Client) ReadBranchConfig(ctx context.Context, branch string) (cfg Branc
 		return
 	}
 
-	cfg.LocalName = branch
 	for _, line := range outputLines(out) {
 		parts := strings.SplitN(line, " ", 2)
 		if len(parts) < 2 {

From 9d490547b8d20f168e9c665172a3a12204e2f497 Mon Sep 17 00:00:00 2001
From: Caleb Brose <5447118+cmbrose@users.noreply.github.com>
Date: Fri, 3 Jan 2025 20:39:12 +0000
Subject: [PATCH 08/25] Alternative: remove LocalBranch from BranchConfig

---
 git/client.go               | 2 --
 git/client_test.go          | 2 +-
 git/objects.go              | 2 --
 pkg/cmd/pr/create/create.go | 6 +++---
 4 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/git/client.go b/git/client.go
index 774ce5e76..1a6d9ae7f 100644
--- a/git/client.go
+++ b/git/client.go
@@ -378,8 +378,6 @@ func (c *Client) lookupCommit(ctx context.Context, sha, format string) ([]byte,
 
 // ReadBranchConfig parses the `branch.BRANCH.(remote|merge|gh-merge-base)` part of git config.
 func (c *Client) ReadBranchConfig(ctx context.Context, branch string) (cfg BranchConfig) {
-	cfg.LocalName = branch
-
 	prefix := regexp.QuoteMeta(fmt.Sprintf("branch.%s.", branch))
 	args := []string{"config", "--get-regexp", fmt.Sprintf("^%s(remote|merge|%s)$", prefix, MergeBaseConfig)}
 	cmd, err := c.Command(ctx, args...)
diff --git a/git/client_test.go b/git/client_test.go
index 0ec4f7334..fff1397f3 100644
--- a/git/client_test.go
+++ b/git/client_test.go
@@ -737,7 +737,7 @@ func TestClientReadBranchConfig(t *testing.T) {
 			name:             "read branch config",
 			cmdStdout:        "branch.trunk.remote origin\nbranch.trunk.merge refs/heads/trunk\nbranch.trunk.gh-merge-base trunk",
 			wantCmdArgs:      `path/to/git config --get-regexp ^branch\.trunk\.(remote|merge|gh-merge-base)$`,
-			wantBranchConfig: BranchConfig{LocalName: "trunk", RemoteName: "origin", MergeRef: "refs/heads/trunk", MergeBase: "trunk"},
+			wantBranchConfig: BranchConfig{RemoteName: "origin", MergeRef: "refs/heads/trunk", MergeBase: "trunk"},
 		},
 	}
 	for _, tt := range tests {
diff --git a/git/objects.go b/git/objects.go
index f371429dd..ae058a01c 100644
--- a/git/objects.go
+++ b/git/objects.go
@@ -71,8 +71,6 @@ type Commit struct {
 }
 
 type BranchConfig struct {
-	// LocalName of the branch.
-	LocalName  string
 	RemoteName string
 	RemoteURL  *url.URL
 	// MergeBase is the optional base branch to target in a new PR if `--base` is not specified.
diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go
index 3bfa768b7..6a1e13849 100644
--- a/pkg/cmd/pr/create/create.go
+++ b/pkg/cmd/pr/create/create.go
@@ -518,7 +518,7 @@ func initDefaultTitleBody(ctx CreateContext, state *shared.IssueMetadataState, u
 	return nil
 }
 
-func determineTrackingBranch(gitClient *git.Client, remotes ghContext.Remotes, headBranchConfig *git.BranchConfig) *git.TrackingRef {
+func determineTrackingBranch(gitClient *git.Client, remotes ghContext.Remotes, localBranchName string, headBranchConfig *git.BranchConfig) *git.TrackingRef {
 	refsForLookup := []string{"HEAD"}
 	var trackingRefs []git.TrackingRef
 
@@ -534,7 +534,7 @@ func determineTrackingBranch(gitClient *git.Client, remotes ghContext.Remotes, h
 	for _, remote := range remotes {
 		tr := git.TrackingRef{
 			RemoteName: remote.Name,
-			BranchName: headBranchConfig.LocalName,
+			BranchName: localBranchName,
 		}
 		trackingRefs = append(trackingRefs, tr)
 		refsForLookup = append(refsForLookup, tr.String())
@@ -647,7 +647,7 @@ func NewCreateContext(opts *CreateOptions) (*CreateContext, error) {
 	headBranchConfig := gitClient.ReadBranchConfig(context.Background(), headBranch)
 	if isPushEnabled {
 		// determine whether the head branch is already pushed to a remote
-		if pushedTo := determineTrackingBranch(gitClient, remotes, &headBranchConfig); pushedTo != nil {
+		if pushedTo := determineTrackingBranch(gitClient, remotes, headBranch, &headBranchConfig); pushedTo != nil {
 			isPushEnabled = false
 			if r, err := remotes.FindByName(pushedTo.RemoteName); err == nil {
 				headRepo = r

From 67749480d566fd2cc268ea68155d132197a5e258 Mon Sep 17 00:00:00 2001
From: Caleb Brose <5447118+cmbrose@users.noreply.github.com>
Date: Fri, 3 Jan 2025 20:45:20 +0000
Subject: [PATCH 09/25] Fix test

---
 pkg/cmd/pr/create/create_test.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go
index 27220d052..9d6584e6c 100644
--- a/pkg/cmd/pr/create/create_test.go
+++ b/pkg/cmd/pr/create/create_test.go
@@ -1719,7 +1719,7 @@ func Test_determineTrackingBranch(t *testing.T) {
 				GitPath: "some/path/git",
 			}
 			headBranchConfig := gitClient.ReadBranchConfig(ctx.Background(), "feature")
-			ref := determineTrackingBranch(gitClient, tt.remotes, &headBranchConfig)
+			ref := determineTrackingBranch(gitClient, tt.remotes, "feature", &headBranchConfig)
 			tt.assert(ref, t)
 		})
 	}

From efec5d92501b582111254cae679afecb157cc3a2 Mon Sep 17 00:00:00 2001
From: Tyler McGoffin <jtmcg@github.com>
Date: Fri, 3 Jan 2025 15:50:54 -0800
Subject: [PATCH 10/25] Fixed test for stdout in non-tty use case of repo fork

---
 pkg/cmd/repo/fork/fork_test.go | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/pkg/cmd/repo/fork/fork_test.go b/pkg/cmd/repo/fork/fork_test.go
index 0252602fe..1f0b9cef1 100644
--- a/pkg/cmd/repo/fork/fork_test.go
+++ b/pkg/cmd/repo/fork/fork_test.go
@@ -390,6 +390,7 @@ func TestRepoFork(t *testing.T) {
 				},
 			},
 			httpStubs: forkPost,
+			wantOut:   "https://github.com/someone/REPO\n",
 		},
 		{
 			name: "implicit nontty remote exists",
@@ -424,11 +425,13 @@ func TestRepoFork(t *testing.T) {
 				cs.Register("git remote rename origin upstream", 0, "")
 				cs.Register(`git remote add origin https://github.com/someone/REPO.git`, 0, "")
 			},
+			wantOut: "https://github.com/someone/REPO\n",
 		},
 		{
 			name:      "implicit nontty no args",
 			opts:      &ForkOptions{},
 			httpStubs: forkPost,
+			wantOut:   "https://github.com/someone/REPO\n",
 		},
 		{
 			name: "passes git flags",
@@ -561,6 +564,7 @@ func TestRepoFork(t *testing.T) {
 				Repository: "OWNER/REPO",
 			},
 			httpStubs: forkPost,
+			wantOut:   "https://github.com/someone/REPO\n",
 		},
 		{
 			name: "repo arg nontty repo already exists",
@@ -604,6 +608,7 @@ func TestRepoFork(t *testing.T) {
 				cs.Register(`git -C REPO fetch upstream`, 0, "")
 				cs.Register(`git -C REPO config --add remote.upstream.gh-resolved base`, 0, "")
 			},
+			wantOut: "https://github.com/someone/REPO\n",
 		},
 		{
 			name: "non tty repo arg with fork-name",
@@ -640,6 +645,7 @@ func TestRepoFork(t *testing.T) {
 					httpmock.StringResponse(renameResult))
 			},
 			wantErrOut: "",
+			wantOut:    "https://github.com/OWNER/REPO\n",
 		},
 		{
 			name: "tty repo arg with fork-name",
@@ -694,6 +700,7 @@ func TestRepoFork(t *testing.T) {
 				cs.Register(`git -C REPO fetch upstream`, 0, "")
 				cs.Register(`git -C REPO config --add remote.upstream.gh-resolved base`, 0, "")
 			},
+			wantOut: "https://github.com/someone/REPO\n",
 		},
 		{
 			name: "does not retry clone if error occurs and exit code is not 128",

From dd32a9cc94f61c823e81568c2822cf4432471a70 Mon Sep 17 00:00:00 2001
From: Josh Soref <2119212+jsoref@users.noreply.github.com>
Date: Sun, 5 Jan 2025 23:18:28 -0500
Subject: [PATCH 11/25] Upgrade generated workflows

Use `cli/gh-extension-precompile@v2`

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
---
 pkg/cmd/extension/ext_tmpls/goBinWorkflow.yml    | 2 +-
 pkg/cmd/extension/ext_tmpls/otherBinWorkflow.yml | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/pkg/cmd/extension/ext_tmpls/goBinWorkflow.yml b/pkg/cmd/extension/ext_tmpls/goBinWorkflow.yml
index 080019c2a..b804448c6 100644
--- a/pkg/cmd/extension/ext_tmpls/goBinWorkflow.yml
+++ b/pkg/cmd/extension/ext_tmpls/goBinWorkflow.yml
@@ -13,7 +13,7 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4
-      - uses: cli/gh-extension-precompile@v1
+      - uses: cli/gh-extension-precompile@v2
         with:
           generate_attestations: true
           go_version_file: go.mod
diff --git a/pkg/cmd/extension/ext_tmpls/otherBinWorkflow.yml b/pkg/cmd/extension/ext_tmpls/otherBinWorkflow.yml
index 78ba05171..4eb99a4a4 100644
--- a/pkg/cmd/extension/ext_tmpls/otherBinWorkflow.yml
+++ b/pkg/cmd/extension/ext_tmpls/otherBinWorkflow.yml
@@ -11,6 +11,6 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4
-      - uses: cli/gh-extension-precompile@v1
+      - uses: cli/gh-extension-precompile@v2
         with:
           build_script_override: "script/build.sh"

From 3695bda6379a1444781110acefe1e0b1306eace3 Mon Sep 17 00:00:00 2001
From: William Martin <williammartin@github.com>
Date: Mon, 6 Jan 2025 16:30:48 +0100
Subject: [PATCH 12/25] Clear up --with-token fine grained PAT usage

---
 pkg/cmd/auth/login/login.go | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go
index ca165a22f..c8011e597 100644
--- a/pkg/cmd/auth/login/login.go
+++ b/pkg/cmd/auth/login/login.go
@@ -69,9 +69,10 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm
 			stored location.
 
 			Alternatively, use %[1]s--with-token%[1]s to pass in a personal access token (classic) on standard input.
-			The minimum required scopes for the token are: %[1]srepo%[1]s, %[1]sread:org%[1]s, and %[1]sgist%[1]s.
-
-			Fine-grained personal access tokens are not supported.
+			The minimum required scopes for the token are: %[1]srepo%[1]s, %[1]sread:org%[1]s, and %[1]sgist%[1]s. Although
+			it is possible to pass a fine-grained personal access token to %[1]s--with-token%[1]s, it should be done with
+			care, as the inherent scoping to certain resources may cause confusing behaviour when interacting with other
+			resources. Favour setting %[1]sGH_TOKEN$%[1]s for fine-grained personal access token usage. 
 
 			Alternatively, gh will use the authentication token found in environment variables.
 			This method is most suitable for "headless" use of gh such as in automation. See

From e7d49fb78c371ba5df66407dbfa9f3aec6e61ea6 Mon Sep 17 00:00:00 2001
From: William Martin <williammartin@github.com>
Date: Mon, 6 Jan 2025 17:21:02 +0100
Subject: [PATCH 13/25] Update pkg/cmd/auth/login/login.go

Co-authored-by: Andy Feller <andyfeller@github.com>
---
 pkg/cmd/auth/login/login.go | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go
index c8011e597..347fcdb6a 100644
--- a/pkg/cmd/auth/login/login.go
+++ b/pkg/cmd/auth/login/login.go
@@ -69,9 +69,9 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm
 			stored location.
 
 			Alternatively, use %[1]s--with-token%[1]s to pass in a personal access token (classic) on standard input.
-			The minimum required scopes for the token are: %[1]srepo%[1]s, %[1]sread:org%[1]s, and %[1]sgist%[1]s. Although
-			it is possible to pass a fine-grained personal access token to %[1]s--with-token%[1]s, it should be done with
-			care, as the inherent scoping to certain resources may cause confusing behaviour when interacting with other
+			The minimum required scopes for the token are: %[1]srepo%[1]s, %[1]sread:org%[1]s, and %[1]sgist%[1]s.
+			Take care when passing a fine-grained personal access token to %[1]s--with-token%[1]s
+			as the inherent scoping to certain resources may cause confusing behaviour when interacting with other
 			resources. Favour setting %[1]sGH_TOKEN$%[1]s for fine-grained personal access token usage. 
 
 			Alternatively, gh will use the authentication token found in environment variables.

From 3ae4e5da20ebd9f9f46342906c0a930b75310d3b Mon Sep 17 00:00:00 2001
From: William Martin <williammartin@github.com>
Date: Mon, 6 Jan 2025 15:14:43 +0100
Subject: [PATCH 14/25] Document and rework pr create tracking branch lookup

---
 git/objects.go              | 12 ++++++++++++
 pkg/cmd/pr/create/create.go | 30 +++++++++++++++++++-----------
 2 files changed, 31 insertions(+), 11 deletions(-)

diff --git a/git/objects.go b/git/objects.go
index ae058a01c..924a3059e 100644
--- a/git/objects.go
+++ b/git/objects.go
@@ -1,6 +1,7 @@
 package git
 
 import (
+	"fmt"
 	"net/url"
 	"strings"
 )
@@ -64,6 +65,17 @@ func (r TrackingRef) String() string {
 	return "refs/remotes/" + r.RemoteName + "/" + r.BranchName
 }
 
+func ParseTrackingRef(text string) (TrackingRef, error) {
+	parts := strings.SplitN(string(text), "/", 4)
+	if len(parts) != 4 {
+		return TrackingRef{}, fmt.Errorf("invalid tracking ref: %s", text)
+	}
+	return TrackingRef{
+		RemoteName: parts[2],
+		BranchName: parts[3],
+	}, nil
+}
+
 type Commit struct {
 	Sha   string
 	Title string
diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go
index 6a1e13849..df62dc889 100644
--- a/pkg/cmd/pr/create/create.go
+++ b/pkg/cmd/pr/create/create.go
@@ -519,15 +519,14 @@ func initDefaultTitleBody(ctx CreateContext, state *shared.IssueMetadataState, u
 }
 
 func determineTrackingBranch(gitClient *git.Client, remotes ghContext.Remotes, localBranchName string, headBranchConfig *git.BranchConfig) *git.TrackingRef {
+	// To try and determine the tracking ref for a local branch, we first construct a collection of refs
+	// that might be tracking, given the current branch's config, and the list of known remotes.
 	refsForLookup := []string{"HEAD"}
-	var trackingRefs []git.TrackingRef
-
-	if headBranchConfig.RemoteName != "" {
+	if headBranchConfig.RemoteName != "" && headBranchConfig.MergeRef != "" {
 		tr := git.TrackingRef{
 			RemoteName: headBranchConfig.RemoteName,
 			BranchName: strings.TrimPrefix(headBranchConfig.MergeRef, "refs/heads/"),
 		}
-		trackingRefs = append(trackingRefs, tr)
 		refsForLookup = append(refsForLookup, tr.String())
 	}
 
@@ -536,22 +535,31 @@ func determineTrackingBranch(gitClient *git.Client, remotes ghContext.Remotes, l
 			RemoteName: remote.Name,
 			BranchName: localBranchName,
 		}
-		trackingRefs = append(trackingRefs, tr)
 		refsForLookup = append(refsForLookup, tr.String())
 	}
 
+	// Then we ask git for details about these refs, for example, refs/remotes/origin/trunk might return a hash
+	// for the remote tracking branch, trunk, for the remote, origin. If there is no ref, the git client returns
+	// no ref information.
+	//
+	// We also first check for the HEAD ref, so that we have the hash of the currently checked out commit.
 	resolvedRefs, _ := gitClient.ShowRefs(context.Background(), refsForLookup)
+
+	// If there is more than one resolved ref, that means that at least one ref was found in addition to the HEAD.
 	if len(resolvedRefs) > 1 {
+		headRef := resolvedRefs[0]
 		for _, r := range resolvedRefs[1:] {
-			if r.Hash != resolvedRefs[0].Hash {
+			// If the hash of the remote ref doesn't match the hash of HEAD then the remote branch is not in the same
+			// state, so it can't be used.
+			if r.Hash != headRef.Hash {
 				continue
 			}
-			for _, tr := range trackingRefs {
-				if tr.String() != r.Name {
-					continue
-				}
-				return &tr
+			// Otherwise we can parse the returned ref into a tracking ref and return that
+			trackingRef, err := git.ParseTrackingRef(r.Name)
+			if err != nil {
+				return nil
 			}
+			return &trackingRef
 		}
 	}
 

From dc077dc09ba68843e973b715c448e376135e6c67 Mon Sep 17 00:00:00 2001
From: William Martin <williammartin@github.com>
Date: Mon, 6 Jan 2025 15:44:34 +0100
Subject: [PATCH 15/25] Panic if tracking ref can't be reconstructed

---
 git/objects.go              | 12 ------------
 pkg/cmd/pr/create/create.go | 16 ++++++++++++----
 2 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/git/objects.go b/git/objects.go
index 924a3059e..ae058a01c 100644
--- a/git/objects.go
+++ b/git/objects.go
@@ -1,7 +1,6 @@
 package git
 
 import (
-	"fmt"
 	"net/url"
 	"strings"
 )
@@ -65,17 +64,6 @@ func (r TrackingRef) String() string {
 	return "refs/remotes/" + r.RemoteName + "/" + r.BranchName
 }
 
-func ParseTrackingRef(text string) (TrackingRef, error) {
-	parts := strings.SplitN(string(text), "/", 4)
-	if len(parts) != 4 {
-		return TrackingRef{}, fmt.Errorf("invalid tracking ref: %s", text)
-	}
-	return TrackingRef{
-		RemoteName: parts[2],
-		BranchName: parts[3],
-	}, nil
-}
-
 type Commit struct {
 	Sha   string
 	Title string
diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go
index df62dc889..d7032714a 100644
--- a/pkg/cmd/pr/create/create.go
+++ b/pkg/cmd/pr/create/create.go
@@ -555,10 +555,7 @@ func determineTrackingBranch(gitClient *git.Client, remotes ghContext.Remotes, l
 				continue
 			}
 			// Otherwise we can parse the returned ref into a tracking ref and return that
-			trackingRef, err := git.ParseTrackingRef(r.Name)
-			if err != nil {
-				return nil
-			}
+			trackingRef := mustParseTrackingRef(r.Name)
 			return &trackingRef
 		}
 	}
@@ -566,6 +563,17 @@ func determineTrackingBranch(gitClient *git.Client, remotes ghContext.Remotes, l
 	return nil
 }
 
+func mustParseTrackingRef(text string) git.TrackingRef {
+	parts := strings.SplitN(string(text), "/", 4)
+	if len(parts) != 4 {
+		panic(fmt.Errorf("invalid tracking ref: %s", text))
+	}
+	return git.TrackingRef{
+		RemoteName: parts[2],
+		BranchName: parts[3],
+	}
+}
+
 func NewIssueState(ctx CreateContext, opts CreateOptions) (*shared.IssueMetadataState, error) {
 	var milestoneTitles []string
 	if opts.Milestone != "" {

From 05764b8114e59411cf41723124d83fbb400d9395 Mon Sep 17 00:00:00 2001
From: William Martin <williammartin@github.com>
Date: Mon, 6 Jan 2025 15:47:10 +0100
Subject: [PATCH 16/25] Don't use pointer for determineTrackingBranch
 branchConfig

---
 pkg/cmd/pr/create/create.go      | 4 ++--
 pkg/cmd/pr/create/create_test.go | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go
index d7032714a..40913db64 100644
--- a/pkg/cmd/pr/create/create.go
+++ b/pkg/cmd/pr/create/create.go
@@ -518,7 +518,7 @@ func initDefaultTitleBody(ctx CreateContext, state *shared.IssueMetadataState, u
 	return nil
 }
 
-func determineTrackingBranch(gitClient *git.Client, remotes ghContext.Remotes, localBranchName string, headBranchConfig *git.BranchConfig) *git.TrackingRef {
+func determineTrackingBranch(gitClient *git.Client, remotes ghContext.Remotes, localBranchName string, headBranchConfig git.BranchConfig) *git.TrackingRef {
 	// To try and determine the tracking ref for a local branch, we first construct a collection of refs
 	// that might be tracking, given the current branch's config, and the list of known remotes.
 	refsForLookup := []string{"HEAD"}
@@ -663,7 +663,7 @@ func NewCreateContext(opts *CreateOptions) (*CreateContext, error) {
 	headBranchConfig := gitClient.ReadBranchConfig(context.Background(), headBranch)
 	if isPushEnabled {
 		// determine whether the head branch is already pushed to a remote
-		if pushedTo := determineTrackingBranch(gitClient, remotes, headBranch, &headBranchConfig); pushedTo != nil {
+		if pushedTo := determineTrackingBranch(gitClient, remotes, headBranch, headBranchConfig); pushedTo != nil {
 			isPushEnabled = false
 			if r, err := remotes.FindByName(pushedTo.RemoteName); err == nil {
 				headRepo = r
diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go
index 9d6584e6c..8d5a1d1b3 100644
--- a/pkg/cmd/pr/create/create_test.go
+++ b/pkg/cmd/pr/create/create_test.go
@@ -1719,7 +1719,7 @@ func Test_determineTrackingBranch(t *testing.T) {
 				GitPath: "some/path/git",
 			}
 			headBranchConfig := gitClient.ReadBranchConfig(ctx.Background(), "feature")
-			ref := determineTrackingBranch(gitClient, tt.remotes, "feature", &headBranchConfig)
+			ref := determineTrackingBranch(gitClient, tt.remotes, "feature", headBranchConfig)
 			tt.assert(ref, t)
 		})
 	}

From 27bd4b2aec09f1242177821ad72a01b01163c998 Mon Sep 17 00:00:00 2001
From: William Martin <williammartin@github.com>
Date: Mon, 6 Jan 2025 15:49:53 +0100
Subject: [PATCH 17/25] Doc determineTrackingBranch

---
 pkg/cmd/pr/create/create.go | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go
index 40913db64..b05c971fe 100644
--- a/pkg/cmd/pr/create/create.go
+++ b/pkg/cmd/pr/create/create.go
@@ -518,6 +518,8 @@ func initDefaultTitleBody(ctx CreateContext, state *shared.IssueMetadataState, u
 	return nil
 }
 
+// determineTrackingBranch is intended to try and find a remote branch on the same commit as the currently checked out
+// HEAD, i.e. the local branch.
 func determineTrackingBranch(gitClient *git.Client, remotes ghContext.Remotes, localBranchName string, headBranchConfig git.BranchConfig) *git.TrackingRef {
 	// To try and determine the tracking ref for a local branch, we first construct a collection of refs
 	// that might be tracking, given the current branch's config, and the list of known remotes.

From b8c167970b70205cba2fd22e2bb2da378229f0d7 Mon Sep 17 00:00:00 2001
From: William Martin <williammartin@github.com>
Date: Mon, 6 Jan 2025 15:55:35 +0100
Subject: [PATCH 18/25] Avoid pointer return from determineTrackingBranch

---
 pkg/cmd/pr/create/create.go      | 17 ++++++++--------
 pkg/cmd/pr/create/create_test.go | 33 +++++++++++++++++++-------------
 2 files changed, 28 insertions(+), 22 deletions(-)

diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go
index b05c971fe..00993e0fa 100644
--- a/pkg/cmd/pr/create/create.go
+++ b/pkg/cmd/pr/create/create.go
@@ -518,9 +518,9 @@ func initDefaultTitleBody(ctx CreateContext, state *shared.IssueMetadataState, u
 	return nil
 }
 
-// determineTrackingBranch is intended to try and find a remote branch on the same commit as the currently checked out
+// tryDetermineTrackingRef is intended to try and find a remote branch on the same commit as the currently checked out
 // HEAD, i.e. the local branch.
-func determineTrackingBranch(gitClient *git.Client, remotes ghContext.Remotes, localBranchName string, headBranchConfig git.BranchConfig) *git.TrackingRef {
+func tryDetermineTrackingRef(gitClient *git.Client, remotes ghContext.Remotes, localBranchName string, headBranchConfig git.BranchConfig) (git.TrackingRef, bool) {
 	// To try and determine the tracking ref for a local branch, we first construct a collection of refs
 	// that might be tracking, given the current branch's config, and the list of known remotes.
 	refsForLookup := []string{"HEAD"}
@@ -557,12 +557,11 @@ func determineTrackingBranch(gitClient *git.Client, remotes ghContext.Remotes, l
 				continue
 			}
 			// Otherwise we can parse the returned ref into a tracking ref and return that
-			trackingRef := mustParseTrackingRef(r.Name)
-			return &trackingRef
+			return mustParseTrackingRef(r.Name), true
 		}
 	}
 
-	return nil
+	return git.TrackingRef{}, false
 }
 
 func mustParseTrackingRef(text string) git.TrackingRef {
@@ -665,14 +664,14 @@ func NewCreateContext(opts *CreateOptions) (*CreateContext, error) {
 	headBranchConfig := gitClient.ReadBranchConfig(context.Background(), headBranch)
 	if isPushEnabled {
 		// determine whether the head branch is already pushed to a remote
-		if pushedTo := determineTrackingBranch(gitClient, remotes, headBranch, headBranchConfig); pushedTo != nil {
+		if trackingRef, found := tryDetermineTrackingRef(gitClient, remotes, headBranch, headBranchConfig); found {
 			isPushEnabled = false
-			if r, err := remotes.FindByName(pushedTo.RemoteName); err == nil {
+			if r, err := remotes.FindByName(trackingRef.RemoteName); err == nil {
 				headRepo = r
 				headRemote = r
-				headBranchLabel = pushedTo.BranchName
+				headBranchLabel = trackingRef.BranchName
 				if !ghrepo.IsSame(baseRepo, headRepo) {
-					headBranchLabel = fmt.Sprintf("%s:%s", headRepo.RepoOwner(), pushedTo.BranchName)
+					headBranchLabel = fmt.Sprintf("%s:%s", headRepo.RepoOwner(), trackingRef.BranchName)
 				}
 			}
 		}
diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go
index 8d5a1d1b3..acb35f557 100644
--- a/pkg/cmd/pr/create/create_test.go
+++ b/pkg/cmd/pr/create/create_test.go
@@ -1622,12 +1622,12 @@ func Test_createRun(t *testing.T) {
 	}
 }
 
-func Test_determineTrackingBranch(t *testing.T) {
+func Test_tryDetermineTrackingRef(t *testing.T) {
 	tests := []struct {
 		name     string
 		cmdStubs func(*run.CommandStubber)
 		remotes  context.Remotes
-		assert   func(ref *git.TrackingRef, t *testing.T)
+		assert   func(ref git.TrackingRef, found bool, t *testing.T)
 	}{
 		{
 			name: "empty",
@@ -1635,8 +1635,9 @@ func Test_determineTrackingBranch(t *testing.T) {
 				cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, "")
 				cs.Register(`git show-ref --verify -- HEAD`, 0, "abc HEAD")
 			},
-			assert: func(ref *git.TrackingRef, t *testing.T) {
-				assert.Nil(t, ref)
+			assert: func(ref git.TrackingRef, found bool, t *testing.T) {
+				assert.Zero(t, ref)
+				assert.False(t, found)
 			},
 		},
 		{
@@ -1655,8 +1656,9 @@ func Test_determineTrackingBranch(t *testing.T) {
 					Repo:   ghrepo.New("octocat", "Spoon-Knife"),
 				},
 			},
-			assert: func(ref *git.TrackingRef, t *testing.T) {
-				assert.Nil(t, ref)
+			assert: func(ref git.TrackingRef, found bool, t *testing.T) {
+				assert.Zero(t, ref)
+				assert.False(t, found)
 			},
 		},
 		{
@@ -1679,9 +1681,12 @@ func Test_determineTrackingBranch(t *testing.T) {
 					Repo:   ghrepo.New("octocat", "Spoon-Knife"),
 				},
 			},
-			assert: func(ref *git.TrackingRef, t *testing.T) {
-				assert.Equal(t, "upstream", ref.RemoteName)
-				assert.Equal(t, "feature", ref.BranchName)
+			assert: func(ref git.TrackingRef, found bool, t *testing.T) {
+				assert.Equal(t, git.TrackingRef{
+					RemoteName: "upstream",
+					BranchName: "feature",
+				}, ref)
+				assert.True(t, found)
 			},
 		},
 		{
@@ -1702,8 +1707,9 @@ func Test_determineTrackingBranch(t *testing.T) {
 					Repo:   ghrepo.New("hubot", "Spoon-Knife"),
 				},
 			},
-			assert: func(ref *git.TrackingRef, t *testing.T) {
-				assert.Nil(t, ref)
+			assert: func(ref git.TrackingRef, found bool, t *testing.T) {
+				assert.Zero(t, ref)
+				assert.False(t, found)
 			},
 		},
 	}
@@ -1719,8 +1725,9 @@ func Test_determineTrackingBranch(t *testing.T) {
 				GitPath: "some/path/git",
 			}
 			headBranchConfig := gitClient.ReadBranchConfig(ctx.Background(), "feature")
-			ref := determineTrackingBranch(gitClient, tt.remotes, "feature", headBranchConfig)
-			tt.assert(ref, t)
+			ref, found := tryDetermineTrackingRef(gitClient, tt.remotes, "feature", headBranchConfig)
+
+			tt.assert(ref, found, t)
 		})
 	}
 }

From 57ba5e56082e442e3e4e856ba8ca4aec703c8499 Mon Sep 17 00:00:00 2001
From: William Martin <williammartin@github.com>
Date: Mon, 6 Jan 2025 15:57:14 +0100
Subject: [PATCH 19/25] Rework tryDetermineTrackingRef tests

---
 pkg/cmd/pr/create/create_test.go | 40 ++++++++++++++------------------
 1 file changed, 17 insertions(+), 23 deletions(-)

diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go
index acb35f557..4785a9d99 100644
--- a/pkg/cmd/pr/create/create_test.go
+++ b/pkg/cmd/pr/create/create_test.go
@@ -1624,10 +1624,11 @@ func Test_createRun(t *testing.T) {
 
 func Test_tryDetermineTrackingRef(t *testing.T) {
 	tests := []struct {
-		name     string
-		cmdStubs func(*run.CommandStubber)
-		remotes  context.Remotes
-		assert   func(ref git.TrackingRef, found bool, t *testing.T)
+		name                string
+		cmdStubs            func(*run.CommandStubber)
+		remotes             context.Remotes
+		expectedTrackingRef git.TrackingRef
+		expectedFound       bool
 	}{
 		{
 			name: "empty",
@@ -1635,10 +1636,8 @@ func Test_tryDetermineTrackingRef(t *testing.T) {
 				cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, "")
 				cs.Register(`git show-ref --verify -- HEAD`, 0, "abc HEAD")
 			},
-			assert: func(ref git.TrackingRef, found bool, t *testing.T) {
-				assert.Zero(t, ref)
-				assert.False(t, found)
-			},
+			expectedTrackingRef: git.TrackingRef{},
+			expectedFound:       false,
 		},
 		{
 			name: "no match",
@@ -1656,10 +1655,8 @@ func Test_tryDetermineTrackingRef(t *testing.T) {
 					Repo:   ghrepo.New("octocat", "Spoon-Knife"),
 				},
 			},
-			assert: func(ref git.TrackingRef, found bool, t *testing.T) {
-				assert.Zero(t, ref)
-				assert.False(t, found)
-			},
+			expectedTrackingRef: git.TrackingRef{},
+			expectedFound:       false,
 		},
 		{
 			name: "match",
@@ -1681,13 +1678,11 @@ func Test_tryDetermineTrackingRef(t *testing.T) {
 					Repo:   ghrepo.New("octocat", "Spoon-Knife"),
 				},
 			},
-			assert: func(ref git.TrackingRef, found bool, t *testing.T) {
-				assert.Equal(t, git.TrackingRef{
-					RemoteName: "upstream",
-					BranchName: "feature",
-				}, ref)
-				assert.True(t, found)
+			expectedTrackingRef: git.TrackingRef{
+				RemoteName: "upstream",
+				BranchName: "feature",
 			},
+			expectedFound: true,
 		},
 		{
 			name: "respect tracking config",
@@ -1707,10 +1702,8 @@ func Test_tryDetermineTrackingRef(t *testing.T) {
 					Repo:   ghrepo.New("hubot", "Spoon-Knife"),
 				},
 			},
-			assert: func(ref git.TrackingRef, found bool, t *testing.T) {
-				assert.Zero(t, ref)
-				assert.False(t, found)
-			},
+			expectedTrackingRef: git.TrackingRef{},
+			expectedFound:       false,
 		},
 	}
 	for _, tt := range tests {
@@ -1727,7 +1720,8 @@ func Test_tryDetermineTrackingRef(t *testing.T) {
 			headBranchConfig := gitClient.ReadBranchConfig(ctx.Background(), "feature")
 			ref, found := tryDetermineTrackingRef(gitClient, tt.remotes, "feature", headBranchConfig)
 
-			tt.assert(ref, found, t)
+			assert.Equal(t, tt.expectedTrackingRef, ref)
+			assert.Equal(t, tt.expectedFound, found)
 		})
 	}
 }

From 62ecb1c84dbb37ea908fa25c7f5e11a27faaf51c Mon Sep 17 00:00:00 2001
From: William Martin <williammartin@github.com>
Date: Mon, 6 Jan 2025 16:10:12 +0100
Subject: [PATCH 20/25] Make tryDetermineTrackingRef tests more respective of
 reality

Though it doesn't really matter, in practice upstream is always going to come before origin.
---
 pkg/cmd/pr/create/create.go      |  4 +++-
 pkg/cmd/pr/create/create_test.go | 26 +++++++++++++-------------
 2 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go
index 00993e0fa..372daf86c 100644
--- a/pkg/cmd/pr/create/create.go
+++ b/pkg/cmd/pr/create/create.go
@@ -519,7 +519,9 @@ func initDefaultTitleBody(ctx CreateContext, state *shared.IssueMetadataState, u
 }
 
 // tryDetermineTrackingRef is intended to try and find a remote branch on the same commit as the currently checked out
-// HEAD, i.e. the local branch.
+// HEAD, i.e. the local branch. If there are multiple branches that might match, the first remote is chosen, which in
+// practice is determined by the sorting algorithm applied much earlier in the process, roughly "upstream", "github", "origin",
+// and then everything else unstably sorted.
 func tryDetermineTrackingRef(gitClient *git.Client, remotes ghContext.Remotes, localBranchName string, headBranchConfig git.BranchConfig) (git.TrackingRef, bool) {
 	// To try and determine the tracking ref for a local branch, we first construct a collection of refs
 	// that might be tracking, given the current branch's config, and the list of known remotes.
diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go
index 4785a9d99..c5127bcc6 100644
--- a/pkg/cmd/pr/create/create_test.go
+++ b/pkg/cmd/pr/create/create_test.go
@@ -1643,17 +1643,17 @@ func Test_tryDetermineTrackingRef(t *testing.T) {
 			name: "no match",
 			cmdStubs: func(cs *run.CommandStubber) {
 				cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, "")
-				cs.Register("git show-ref --verify -- HEAD refs/remotes/origin/feature refs/remotes/upstream/feature", 0, "abc HEAD\nbca refs/remotes/origin/feature")
+				cs.Register("git show-ref --verify -- HEAD refs/remotes/upstream/feature refs/remotes/origin/feature", 0, "abc HEAD\nbca refs/remotes/upstream/feature")
 			},
 			remotes: context.Remotes{
-				&context.Remote{
-					Remote: &git.Remote{Name: "origin"},
-					Repo:   ghrepo.New("hubot", "Spoon-Knife"),
-				},
 				&context.Remote{
 					Remote: &git.Remote{Name: "upstream"},
 					Repo:   ghrepo.New("octocat", "Spoon-Knife"),
 				},
+				&context.Remote{
+					Remote: &git.Remote{Name: "origin"},
+					Repo:   ghrepo.New("hubot", "Spoon-Knife"),
+				},
 			},
 			expectedTrackingRef: git.TrackingRef{},
 			expectedFound:       false,
@@ -1662,24 +1662,24 @@ func Test_tryDetermineTrackingRef(t *testing.T) {
 			name: "match",
 			cmdStubs: func(cs *run.CommandStubber) {
 				cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, "")
-				cs.Register(`git show-ref --verify -- HEAD refs/remotes/origin/feature refs/remotes/upstream/feature$`, 0, heredoc.Doc(`
+				cs.Register(`git show-ref --verify -- HEAD refs/remotes/upstream/feature refs/remotes/origin/feature$`, 0, heredoc.Doc(`
 		deadbeef HEAD
-		deadb00f refs/remotes/origin/feature
-		deadbeef refs/remotes/upstream/feature
+		deadb00f refs/remotes/upstream/feature
+		deadbeef refs/remotes/origin/feature
 	`))
 			},
 			remotes: context.Remotes{
-				&context.Remote{
-					Remote: &git.Remote{Name: "origin"},
-					Repo:   ghrepo.New("hubot", "Spoon-Knife"),
-				},
 				&context.Remote{
 					Remote: &git.Remote{Name: "upstream"},
 					Repo:   ghrepo.New("octocat", "Spoon-Knife"),
 				},
+				&context.Remote{
+					Remote: &git.Remote{Name: "origin"},
+					Repo:   ghrepo.New("hubot", "Spoon-Knife"),
+				},
 			},
 			expectedTrackingRef: git.TrackingRef{
-				RemoteName: "upstream",
+				RemoteName: "origin",
 				BranchName: "feature",
 			},
 			expectedFound: true,

From 8b5073d6172f9cfe4ad4c8ba157ee35bff3f6ed6 Mon Sep 17 00:00:00 2001
From: William Martin <williammartin@github.com>
Date: Mon, 6 Jan 2025 16:58:35 +0100
Subject: [PATCH 21/25] Move trackingRef into pr create package

---
 git/objects.go                   | 10 ------
 pkg/cmd/pr/create/create.go      | 54 +++++++++++++++++++-------------
 pkg/cmd/pr/create/create_test.go | 14 ++++-----
 3 files changed, 39 insertions(+), 39 deletions(-)

diff --git a/git/objects.go b/git/objects.go
index ae058a01c..c09683042 100644
--- a/git/objects.go
+++ b/git/objects.go
@@ -54,16 +54,6 @@ type Ref struct {
 	Name string
 }
 
-// TrackingRef represents a ref for a remote tracking branch.
-type TrackingRef struct {
-	RemoteName string
-	BranchName string
-}
-
-func (r TrackingRef) String() string {
-	return "refs/remotes/" + r.RemoteName + "/" + r.BranchName
-}
-
 type Commit struct {
 	Sha   string
 	Title string
diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go
index 372daf86c..46a4e284e 100644
--- a/pkg/cmd/pr/create/create.go
+++ b/pkg/cmd/pr/create/create.go
@@ -518,26 +518,47 @@ func initDefaultTitleBody(ctx CreateContext, state *shared.IssueMetadataState, u
 	return nil
 }
 
+// trackingRef represents a ref for a remote tracking branch.
+type trackingRef struct {
+	remoteName string
+	branchName string
+}
+
+func (r trackingRef) String() string {
+	return "refs/remotes/" + r.remoteName + "/" + r.branchName
+}
+
+func mustParseTrackingRef(text string) trackingRef {
+	parts := strings.SplitN(string(text), "/", 4)
+	if len(parts) != 4 {
+		panic(fmt.Errorf("invalid tracking ref: %s", text))
+	}
+	return trackingRef{
+		remoteName: parts[2],
+		branchName: parts[3],
+	}
+}
+
 // tryDetermineTrackingRef is intended to try and find a remote branch on the same commit as the currently checked out
 // HEAD, i.e. the local branch. If there are multiple branches that might match, the first remote is chosen, which in
 // practice is determined by the sorting algorithm applied much earlier in the process, roughly "upstream", "github", "origin",
 // and then everything else unstably sorted.
-func tryDetermineTrackingRef(gitClient *git.Client, remotes ghContext.Remotes, localBranchName string, headBranchConfig git.BranchConfig) (git.TrackingRef, bool) {
+func tryDetermineTrackingRef(gitClient *git.Client, remotes ghContext.Remotes, localBranchName string, headBranchConfig git.BranchConfig) (trackingRef, bool) {
 	// To try and determine the tracking ref for a local branch, we first construct a collection of refs
 	// that might be tracking, given the current branch's config, and the list of known remotes.
 	refsForLookup := []string{"HEAD"}
 	if headBranchConfig.RemoteName != "" && headBranchConfig.MergeRef != "" {
-		tr := git.TrackingRef{
-			RemoteName: headBranchConfig.RemoteName,
-			BranchName: strings.TrimPrefix(headBranchConfig.MergeRef, "refs/heads/"),
+		tr := trackingRef{
+			remoteName: headBranchConfig.RemoteName,
+			branchName: strings.TrimPrefix(headBranchConfig.MergeRef, "refs/heads/"),
 		}
 		refsForLookup = append(refsForLookup, tr.String())
 	}
 
 	for _, remote := range remotes {
-		tr := git.TrackingRef{
-			RemoteName: remote.Name,
-			BranchName: localBranchName,
+		tr := trackingRef{
+			remoteName: remote.Name,
+			branchName: localBranchName,
 		}
 		refsForLookup = append(refsForLookup, tr.String())
 	}
@@ -563,18 +584,7 @@ func tryDetermineTrackingRef(gitClient *git.Client, remotes ghContext.Remotes, l
 		}
 	}
 
-	return git.TrackingRef{}, false
-}
-
-func mustParseTrackingRef(text string) git.TrackingRef {
-	parts := strings.SplitN(string(text), "/", 4)
-	if len(parts) != 4 {
-		panic(fmt.Errorf("invalid tracking ref: %s", text))
-	}
-	return git.TrackingRef{
-		RemoteName: parts[2],
-		BranchName: parts[3],
-	}
+	return trackingRef{}, false
 }
 
 func NewIssueState(ctx CreateContext, opts CreateOptions) (*shared.IssueMetadataState, error) {
@@ -668,12 +678,12 @@ func NewCreateContext(opts *CreateOptions) (*CreateContext, error) {
 		// determine whether the head branch is already pushed to a remote
 		if trackingRef, found := tryDetermineTrackingRef(gitClient, remotes, headBranch, headBranchConfig); found {
 			isPushEnabled = false
-			if r, err := remotes.FindByName(trackingRef.RemoteName); err == nil {
+			if r, err := remotes.FindByName(trackingRef.remoteName); err == nil {
 				headRepo = r
 				headRemote = r
-				headBranchLabel = trackingRef.BranchName
+				headBranchLabel = trackingRef.branchName
 				if !ghrepo.IsSame(baseRepo, headRepo) {
-					headBranchLabel = fmt.Sprintf("%s:%s", headRepo.RepoOwner(), trackingRef.BranchName)
+					headBranchLabel = fmt.Sprintf("%s:%s", headRepo.RepoOwner(), trackingRef.branchName)
 				}
 			}
 		}
diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go
index c5127bcc6..6c0ff11e2 100644
--- a/pkg/cmd/pr/create/create_test.go
+++ b/pkg/cmd/pr/create/create_test.go
@@ -1627,7 +1627,7 @@ func Test_tryDetermineTrackingRef(t *testing.T) {
 		name                string
 		cmdStubs            func(*run.CommandStubber)
 		remotes             context.Remotes
-		expectedTrackingRef git.TrackingRef
+		expectedTrackingRef trackingRef
 		expectedFound       bool
 	}{
 		{
@@ -1636,7 +1636,7 @@ func Test_tryDetermineTrackingRef(t *testing.T) {
 				cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, "")
 				cs.Register(`git show-ref --verify -- HEAD`, 0, "abc HEAD")
 			},
-			expectedTrackingRef: git.TrackingRef{},
+			expectedTrackingRef: trackingRef{},
 			expectedFound:       false,
 		},
 		{
@@ -1655,7 +1655,7 @@ func Test_tryDetermineTrackingRef(t *testing.T) {
 					Repo:   ghrepo.New("hubot", "Spoon-Knife"),
 				},
 			},
-			expectedTrackingRef: git.TrackingRef{},
+			expectedTrackingRef: trackingRef{},
 			expectedFound:       false,
 		},
 		{
@@ -1678,9 +1678,9 @@ func Test_tryDetermineTrackingRef(t *testing.T) {
 					Repo:   ghrepo.New("hubot", "Spoon-Knife"),
 				},
 			},
-			expectedTrackingRef: git.TrackingRef{
-				RemoteName: "origin",
-				BranchName: "feature",
+			expectedTrackingRef: trackingRef{
+				remoteName: "origin",
+				branchName: "feature",
 			},
 			expectedFound: true,
 		},
@@ -1702,7 +1702,7 @@ func Test_tryDetermineTrackingRef(t *testing.T) {
 					Repo:   ghrepo.New("hubot", "Spoon-Knife"),
 				},
 			},
-			expectedTrackingRef: git.TrackingRef{},
+			expectedTrackingRef: trackingRef{},
 			expectedFound:       false,
 		},
 	}

From c3b41e87b89aad1f8ea3fcbe4c93c08c3a50b3f9 Mon Sep 17 00:00:00 2001
From: William Martin <williammartin@github.com>
Date: Mon, 6 Jan 2025 17:00:16 +0100
Subject: [PATCH 22/25] Panic mustParseTrackingRef if format is incorrect

---
 pkg/cmd/pr/create/create.go | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go
index 46a4e284e..db160b419 100644
--- a/pkg/cmd/pr/create/create.go
+++ b/pkg/cmd/pr/create/create.go
@@ -530,9 +530,16 @@ func (r trackingRef) String() string {
 
 func mustParseTrackingRef(text string) trackingRef {
 	parts := strings.SplitN(string(text), "/", 4)
+	// The only place this is called is tryDetermineTrackingRef, where we are reconstructing
+	// the same tracking ref we passed in. If it doesn't match the expected format, this is a
+	// programmer error we want to know about, so it's ok to panic.
 	if len(parts) != 4 {
-		panic(fmt.Errorf("invalid tracking ref: %s", text))
+		panic(fmt.Errorf("tracking ref should have four parts: %s", text))
 	}
+	if parts[0] != "refs" || parts[1] != "remotes" {
+		panic(fmt.Errorf("tracking ref should start with refs/remotes/: %s", text))
+	}
+
 	return trackingRef{
 		remoteName: parts[2],
 		branchName: parts[3],

From 757966ca7cdd222b24c04eca2db8321db61a464f Mon Sep 17 00:00:00 2001
From: Andy Feller <andyfeller@github.com>
Date: Mon, 6 Jan 2025 14:27:03 -0500
Subject: [PATCH 23/25] Bump cli/go-gh for indirect security vulnerability

---
 go.mod | 2 +-
 go.sum | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/go.mod b/go.mod
index 98ee0280f..f2f1ce812 100644
--- a/go.mod
+++ b/go.mod
@@ -11,7 +11,7 @@ require (
 	github.com/cenkalti/backoff/v4 v4.3.0
 	github.com/charmbracelet/glamour v0.7.0
 	github.com/charmbracelet/lipgloss v0.10.1-0.20240413172830-d0be07ea6b9c
-	github.com/cli/go-gh/v2 v2.11.1
+	github.com/cli/go-gh/v2 v2.11.2
 	github.com/cli/go-internal v0.0.0-20241025142207-6c48bcd5ce24
 	github.com/cli/oauth v1.1.1
 	github.com/cli/safeexec v1.0.1
diff --git a/go.sum b/go.sum
index 49e104d07..1b48f0c27 100644
--- a/go.sum
+++ b/go.sum
@@ -95,8 +95,8 @@ github.com/charmbracelet/x/exp/term v0.0.0-20240425164147-ba2a9512b05f/go.mod h1
 github.com/cli/browser v1.0.0/go.mod h1:IEWkHYbLjkhtjwwWlwTHW2lGxeS5gezEQBMLTwDHf5Q=
 github.com/cli/browser v1.3.0 h1:LejqCrpWr+1pRqmEPDGnTZOjsMe7sehifLynZJuqJpo=
 github.com/cli/browser v1.3.0/go.mod h1:HH8s+fOAxjhQoBUAsKuPCbqUuxZDhQ2/aD+SzsEfBTk=
-github.com/cli/go-gh/v2 v2.11.1 h1:amAyfqMWQTBdue8iTmDUegGZK7c8kk6WCxD9l/wLtGI=
-github.com/cli/go-gh/v2 v2.11.1/go.mod h1:MeRoKzXff3ygHu7zP+NVTT+imcHW6p3tpuxHAzRM2xE=
+github.com/cli/go-gh/v2 v2.11.2 h1:oad1+sESTPNTiTvh3I3t8UmxuovNDxhwLzeMHk45Q9w=
+github.com/cli/go-gh/v2 v2.11.2/go.mod h1:vVFhi3TfjseIW26ED9itAR8gQK0aVThTm8sYrsZ5QTI=
 github.com/cli/go-internal v0.0.0-20241025142207-6c48bcd5ce24 h1:QDrhR4JA2n3ij9YQN0u5ZeuvRIIvsUGmf5yPlTS0w8E=
 github.com/cli/go-internal v0.0.0-20241025142207-6c48bcd5ce24/go.mod h1:rr9GNING0onuVw8MnracQHn7PcchnFlP882Y0II2KZk=
 github.com/cli/oauth v1.1.1 h1:459gD3hSjlKX9B1uXBuiAMdpXBUQ9QGf/NDcCpoQxPs=

From 58ca3c69bbf4a84828773e844332f9026fa619d8 Mon Sep 17 00:00:00 2001
From: Tyler McGoffin <jtmcg@github.com>
Date: Tue, 7 Jan 2025 10:36:15 -0800
Subject: [PATCH 24/25] Update triage.md to reflect FR experiment outcome

---
 docs/triage.md | 91 +++++++++++++++++++++++++-------------------------
 1 file changed, 46 insertions(+), 45 deletions(-)

diff --git a/docs/triage.md b/docs/triage.md
index 9d54fcfe7..468cc41cc 100644
--- a/docs/triage.md
+++ b/docs/triage.md
@@ -1,27 +1,51 @@
 # Triage role
 
-As we get more issues and pull requests opened on the GitHub CLI, we've decided on a weekly rotation
-triage role. The initial expectation is that the person in the role for the week spends no more than
-2 hours a day on this work; we can refine that as needed.
+As we get more issues and pull requests opened on the GitHub CLI, we've decided on a weekly rotation triage role as defined by our First Responder (FR) rotation. The primary responsibility of the FR during that week is to triage incoming issues from the Open Source community, as defined below. An issue is considered "triaged" when the `needs-triage` label is removed.
 
-## Expectations for incoming issues
+## Expectations for triaging incoming issues
 
-Review and label [open issues missing either the `enhancement`, `bug`, or `docs` label](https://github.com/cli/cli/issues?q=is%3Aopen+is%3Aissue+-label%3Abug%2Cenhancement%2Cdocs+).
+Review and label [open issues missing either the `enhancement`, `bug`, or `docs` label](https://github.com/cli/cli/issues?q=is%3Aopen+is%3Aissue+-label%3Abug%2Cenhancement%2Cdocs+) and the label(s) corresponding to the command space prefixed with `gh-`, such as `gh-pr` for the `gh pr` command set, or `gh-extension` for the `gh extension` command set. 
 
-Any issue that is accepted to be done as either an `enhancement`, `bug`, or `docs` requires explicit Acceptance Criteria in a comment on the issue before `needs-triage` label is removed.
+Then, engage with the issue and community with the goal to remove the `needs-triage` label from the issue. The heuristics for triaging the different issue types are as follow:
 
-To be considered triaged, `enhancement` issues require at least one of the following additional labels:
+### Bugs
 
-- `core`: reserved for the core CLI team
-- `help wanted`: signal that we are accepting contributions for this
-- `discuss`: add to our team's queue to discuss during a sync
-- `needs-investigation`: work that requires a mystery be solved by the core team before it can move forward
-- `needs-user-input`: we need more information from our users before the task can move forward
+To be considered triaged, `bug` issues require the following:
 
-To be considered triaged, `bug` issues require a severity label: one of `p1`, `p2`, or `p3`, which are defined as follows:
- - `p1`: Affects a large population and inhibits work
- - `p2`: Affects more than a few users but doesn't prevent core functions
- - `p3`: Affects a small number of users or is largely cosmetic
+- A severity label `p1`, `p2`, and `p3`
+- Clearly defined Acceptance Criteria, added to the Issue as a standalone comment (see [example](https://github.com/cli/cli/issues/9469#issuecomment-2292315743))
+
+#### Bug severities
+
+| Severity | Description |
+| - | - |
+| `p1` | Affects a large population and inhibits work |
+| `p2` | Affects more than a few users but doesn't prevent core functions |
+| `p3` | Affects a small number of users or is largely cosmetic |
+
+### Enhancements
+
+To be considered triaged, `enhancement` issues require either
+
+- Clearly defined Acceptance Criteria as above 
+- The `needs-investigation` or `needs-design` label with a clearly defined set of open questions to be investigated.
+
+### Docs
+
+To be considered triaged, `docs` issues require clearly defined Acceptance Criteria, as defined above 
+
+## Additional triaging processes and labels
+
+Before removing the `needs-triage` label, consider adding any of the following labels below.
+
+| Label | Description |
+| - | - |
+| `discuss` | Some issues require discussion with the internal team. Adding this label will automatically open up an internal discussion with the team to facilitate this discussion. |
+| `core` |  Defines what we would like to do internally. We tend to lean towards `help wanted` by default, and adding `core` should be reserved for trickier issues or implementations we have strong opinions/preferences about. |
+| `good first issue` | Used to denote when an issue may be a good candidate for a first-time contributor to the CLI. These are usually small and well defined issues. |
+| `help wanted` |  Defines what we feel the community could solve should they care to contribute, respectively. We tend to lean towards `help wanted` by default, and adding `core` should be reserved for trickier issues or implementations we have strong opinions/preferences about. |
+| `invalid` | Added to spam and abusive issues. |
+| `needs-user-input` | Some issues require a lot of back and forth with a contributor before they can be successfully triaged. After asking any contributors for more information, add this label so it is clear that the issue has been responded to and we are waiting on the user. |
 
 ## Expectations for community pull requests
 
@@ -29,36 +53,13 @@ All incoming pull requests are assigned to one of the engineers for review on a
 The person in a triage role for a week could take a glance at these pull requests, mostly to see whether
 the changeset is feasible and to allow the associated CI run for new contributors.
 
-## Issue triage flowchart
+## Spam and abuse
 
-- can this be closed outright?
-  - e.g. spam/junk
-  - add the `invalid` label
-  - close without comment
-- do we not want to do it?
-  - e.g. we have already discussed not wanting to do or it's a duplicate issue
-  - add the appropriate label (e.g. `duplicate`)
-  - comment and close
-- do we want external contribution for this?
-  - e.g. the task is relatively straightforward, but the core team does not have the bandwidth to take it on
-  - ensure that the thread contains all the context necessary for someone new to pick this up
-  - add the `help wanted` label
-  - consider adding `good first issue` label
-- do we want external design contribution for this?
-  - e.g. the task is worthwhile, but needs design work to flesh out the details before implementation and the core team does not have the bandwidth to take it on
-  - ensure that the thread contains all the context necessary for someone new to pick this up
-  - add both the `help wanted` and `needs-design` labels
-- do we want to do it?
-  - add the `core` label
-  - comment acknowledging that
-- is it intriguing, but requires discussion?
-  - Add the `discuss` label
-  - Add the `needs-investigation` label if engineering research is required before action can be taken
-- does it need more info from the issue author?
-  - ask the user for details
-  - add the `needs-user-input` label
-- is it a usage/support question?
-  - Convert the Issue to a Discussion
+The primary goal of triaging spam and abuse is to remove distracting and offensive content from our community.
+
+We get a lot of spam. Whenever you determine an issue as spam, add the `invalid` label and close it as "won't do". For spammy comments, simply mark them as spam using GitHub's built-in spam feature.
+
+Abusive contributions are defined by our [Code of Conduct](../.github/CODE-OF-CONDUCT.md). Any contribution you determine abusive should be removed. Repeat offenses or particularly offensive abuse should be reported using GitHub's reporting features. If an entire issue is abusive, label it as `invalid` and close as "won't do".
 
 ## Weekly PR audit
 

From 47c391b24682735d341e4b7e59d8bc0651da9069 Mon Sep 17 00:00:00 2001
From: Tyler McGoffin <jtmcg@github.com>
Date: Tue, 7 Jan 2025 15:42:17 -0800
Subject: [PATCH 25/25] PR review edits

---
 docs/triage.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/docs/triage.md b/docs/triage.md
index 468cc41cc..7f8bbbd7b 100644
--- a/docs/triage.md
+++ b/docs/triage.md
@@ -45,7 +45,7 @@ Before removing the `needs-triage` label, consider adding any of the following l
 | `good first issue` | Used to denote when an issue may be a good candidate for a first-time contributor to the CLI. These are usually small and well defined issues. |
 | `help wanted` |  Defines what we feel the community could solve should they care to contribute, respectively. We tend to lean towards `help wanted` by default, and adding `core` should be reserved for trickier issues or implementations we have strong opinions/preferences about. |
 | `invalid` | Added to spam and abusive issues. |
-| `needs-user-input` | Some issues require a lot of back and forth with a contributor before they can be successfully triaged. After asking any contributors for more information, add this label so it is clear that the issue has been responded to and we are waiting on the user. |
+| `needs-user-input` | After asking any contributors for more information, add this label so it is clear that the issue has been responded to and we are waiting on the user. |
 
 ## Expectations for community pull requests
 
@@ -59,7 +59,7 @@ The primary goal of triaging spam and abuse is to remove distracting and offensi
 
 We get a lot of spam. Whenever you determine an issue as spam, add the `invalid` label and close it as "won't do". For spammy comments, simply mark them as spam using GitHub's built-in spam feature.
 
-Abusive contributions are defined by our [Code of Conduct](../.github/CODE-OF-CONDUCT.md). Any contribution you determine abusive should be removed. Repeat offenses or particularly offensive abuse should be reported using GitHub's reporting features. If an entire issue is abusive, label it as `invalid` and close as "won't do".
+Abusive contributions are defined by our [Code of Conduct](../.github/CODE-OF-CONDUCT.md). Any contribution you determine abusive should be removed. Repeat offenses or particularly offensive abuse should be reported using GitHub's reporting features and the user blocked. If an entire issue is abusive, label it as `invalid` and close as "won't do".
 
 ## Weekly PR audit