From b451c8498e5aa6590ad41ac1e87d7749d6e4b306 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 3 Mar 2026 02:10:20 +0000 Subject: [PATCH 1/4] Initial plan From d90dea11f743b8974c00b4f9e736d6cfee0b7097 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 3 Mar 2026 02:31:23 +0000 Subject: [PATCH 2/4] refactor: extract duplicate functions to shared packages (stringutil, gitutil)" - Add NormalizeGitHubHostURL to pkg/stringutil/urls.go - Add ExtractBaseRepo and FindGitRoot to pkg/gitutil/gitutil.go - Update pkg/parser/github.go and pkg/cli/github.go to use stringutil.NormalizeGitHubHostURL - Update pkg/workflow/action_resolver.go and pkg/cli/update_actions.go to use gitutil.ExtractBaseRepo - Update pkg/cli/git.go to delegate findGitRoot to gitutil.FindGitRoot - Update pkg/workflow/git_helpers.go to delegate findGitRoot to gitutil.FindGitRoot - Update test files to use the shared functions Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/git.go | 12 ++---------- pkg/cli/github.go | 16 ++------------- pkg/cli/update_actions.go | 16 ++------------- pkg/cli/update_actions_test.go | 6 ++++-- pkg/gitutil/gitutil.go | 29 ++++++++++++++++++++++++++++ pkg/parser/github.go | 16 ++------------- pkg/stringutil/urls.go | 14 ++++++++++++++ pkg/workflow/action_resolver.go | 17 +++------------- pkg/workflow/action_resolver_test.go | 5 +++-- pkg/workflow/git_helpers.go | 5 ++--- 10 files changed, 63 insertions(+), 73 deletions(-) diff --git a/pkg/cli/git.go b/pkg/cli/git.go index df4c87c99de..725c4d2aff8 100644 --- a/pkg/cli/git.go +++ b/pkg/cli/git.go @@ -10,6 +10,7 @@ import ( "github.com/github/gh-aw/pkg/console" "github.com/github/gh-aw/pkg/fileutil" + "github.com/github/gh-aw/pkg/gitutil" "github.com/github/gh-aw/pkg/logger" ) @@ -22,16 +23,7 @@ func isGitRepo() bool { // findGitRoot finds the root directory of the git repository func findGitRoot() (string, error) { - gitLog.Print("Finding git root directory") - cmd := exec.Command("git", "rev-parse", "--show-toplevel") - output, err := cmd.Output() - if err != nil { - gitLog.Printf("Failed to find git root: %v", err) - return "", fmt.Errorf("not in a git repository or git command failed: %w", err) - } - gitRoot := strings.TrimSpace(string(output)) - gitLog.Printf("Found git root: %s", gitRoot) - return gitRoot, nil + return gitutil.FindGitRoot() } // findGitRootForPath finds the root directory of the git repository containing the specified path diff --git a/pkg/cli/github.go b/pkg/cli/github.go index 33dba0315ef..0ea0f8a0ebf 100644 --- a/pkg/cli/github.go +++ b/pkg/cli/github.go @@ -6,6 +6,7 @@ import ( "github.com/github/gh-aw/pkg/constants" "github.com/github/gh-aw/pkg/logger" + "github.com/github/gh-aw/pkg/stringutil" ) var githubLog = logger.New("cli:github") @@ -25,7 +26,7 @@ func getGitHubHost() string { for _, envVar := range envVars { if value := os.Getenv(envVar); value != "" { githubLog.Printf("Resolved GitHub host from %s: %s", envVar, value) - return normalizeGitHubHostURL(value) + return stringutil.NormalizeGitHubHostURL(value) } } @@ -34,19 +35,6 @@ func getGitHubHost() string { return defaultHost } -// normalizeGitHubHostURL ensures the host URL has https:// scheme and no trailing slashes -func normalizeGitHubHostURL(rawHostURL string) string { - // Remove all trailing slashes - normalized := strings.TrimRight(rawHostURL, "/") - - // Add https:// scheme if no scheme is present - if !strings.HasPrefix(normalized, "https://") && !strings.HasPrefix(normalized, "http://") { - normalized = "https://" + normalized - } - - return normalized -} - // getGitHubHostForRepo returns the GitHub host URL for a specific repository. // The gh-aw repository (github/gh-aw) always uses public GitHub (https://github.com) // regardless of enterprise GitHub host settings, since gh-aw itself is only available diff --git a/pkg/cli/update_actions.go b/pkg/cli/update_actions.go index 04ec324cce7..8a8cacdda48 100644 --- a/pkg/cli/update_actions.go +++ b/pkg/cli/update_actions.go @@ -17,18 +17,6 @@ import ( "github.com/github/gh-aw/pkg/workflow" ) -// extractBaseRepo extracts the base repository (owner/repo) from an action path -// that may include subfolders (e.g., "actions/cache/restore" -> "actions/cache") -func extractBaseRepo(actionPath string) string { - parts := strings.Split(actionPath, "/") - if len(parts) >= 2 { - // Return owner/repo (first two segments) - return parts[0] + "/" + parts[1] - } - // If less than 2 parts, return as-is (shouldn't happen in practice) - return actionPath -} - // isCoreAction returns true if the repo is a GitHub-maintained core action (actions/* org). // Core actions are always updated to the latest major version without requiring --major. func isCoreAction(repo string) bool { @@ -172,7 +160,7 @@ func getLatestActionRelease(repo, currentVersion string, allowMajor, verbose boo updateLog.Printf("Getting latest release for %s@%s (allowMajor=%v)", repo, currentVersion, allowMajor) // Extract base repository (e.g., "actions/cache/restore" -> "actions/cache") - baseRepo := extractBaseRepo(repo) + baseRepo := gitutil.ExtractBaseRepo(repo) updateLog.Printf("Using base repository: %s for action: %s", baseRepo, repo) // Use gh CLI to get releases @@ -282,7 +270,7 @@ func getLatestActionReleaseViaGit(repo, currentVersion string, allowMajor, verbo } // Extract base repository (e.g., "actions/cache/restore" -> "actions/cache") - baseRepo := extractBaseRepo(repo) + baseRepo := gitutil.ExtractBaseRepo(repo) updateLog.Printf("Using base repository: %s for action: %s (git fallback)", baseRepo, repo) githubHost := getGitHubHostForRepo(baseRepo) diff --git a/pkg/cli/update_actions_test.go b/pkg/cli/update_actions_test.go index 21edb94c63e..727503ffc97 100644 --- a/pkg/cli/update_actions_test.go +++ b/pkg/cli/update_actions_test.go @@ -5,6 +5,8 @@ package cli import ( "encoding/json" "testing" + + "github.com/github/gh-aw/pkg/gitutil" ) func TestActionKeyVersionConsistency(t *testing.T) { @@ -146,9 +148,9 @@ func TestExtractBaseRepo(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := extractBaseRepo(tt.actionPath) + got := gitutil.ExtractBaseRepo(tt.actionPath) if got != tt.want { - t.Errorf("extractBaseRepo(%q) = %q, want %q", tt.actionPath, got, tt.want) + t.Errorf("gitutil.ExtractBaseRepo(%q) = %q, want %q", tt.actionPath, got, tt.want) } }) } diff --git a/pkg/gitutil/gitutil.go b/pkg/gitutil/gitutil.go index 8f462c44e50..f4f64d5cc59 100644 --- a/pkg/gitutil/gitutil.go +++ b/pkg/gitutil/gitutil.go @@ -1,6 +1,8 @@ package gitutil import ( + "fmt" + "os/exec" "strings" "github.com/github/gh-aw/pkg/logger" @@ -39,3 +41,30 @@ func IsHexString(s string) bool { } return true } + +// ExtractBaseRepo extracts the base repository (owner/repo) from a repository path +// that may include subfolders. +// For "actions/checkout" -> "actions/checkout" +// For "github/codeql-action/upload-sarif" -> "github/codeql-action" +func ExtractBaseRepo(repoPath string) string { + parts := strings.Split(repoPath, "/") + if len(parts) >= 2 { + return parts[0] + "/" + parts[1] + } + return repoPath +} + +// FindGitRoot finds the root directory of the git repository. +// Returns an error if not in a git repository or if the git command fails. +func FindGitRoot() (string, error) { + log.Print("Finding git root directory") + cmd := exec.Command("git", "rev-parse", "--show-toplevel") + output, err := cmd.Output() + if err != nil { + log.Printf("Failed to find git root: %v", err) + return "", fmt.Errorf("not in a git repository or git command failed: %w", err) + } + gitRoot := strings.TrimSpace(string(output)) + log.Printf("Found git root: %s", gitRoot) + return gitRoot, nil +} diff --git a/pkg/parser/github.go b/pkg/parser/github.go index fc5bcc30965..5e69f673c56 100644 --- a/pkg/parser/github.go +++ b/pkg/parser/github.go @@ -11,6 +11,7 @@ import ( "github.com/github/gh-aw/pkg/constants" "github.com/github/gh-aw/pkg/logger" + "github.com/github/gh-aw/pkg/stringutil" ) var githubLog = logger.New("parser:github") @@ -30,7 +31,7 @@ func GetGitHubHost() string { for _, envVar := range envVars { if value := os.Getenv(envVar); value != "" { githubLog.Printf("Resolved GitHub host from %s: %s", envVar, value) - return normalizeGitHubHostURL(value) + return stringutil.NormalizeGitHubHostURL(value) } } @@ -39,19 +40,6 @@ func GetGitHubHost() string { return defaultHost } -// normalizeGitHubHostURL ensures the host URL has https:// scheme and no trailing slashes -func normalizeGitHubHostURL(rawHostURL string) string { - // Remove all trailing slashes - normalized := strings.TrimRight(rawHostURL, "/") - - // Add https:// scheme if no scheme is present - if !strings.HasPrefix(normalized, "https://") && !strings.HasPrefix(normalized, "http://") { - normalized = "https://" + normalized - } - - return normalized -} - // GetGitHubHostForRepo returns the GitHub host URL for a specific repository. // The gh-aw repository (github/gh-aw) always uses public GitHub (https://github.com) // regardless of enterprise GitHub host settings, since gh-aw itself is only available diff --git a/pkg/stringutil/urls.go b/pkg/stringutil/urls.go index 9b48f75d66c..34c182a1c88 100644 --- a/pkg/stringutil/urls.go +++ b/pkg/stringutil/urls.go @@ -5,6 +5,20 @@ import ( "strings" ) +// NormalizeGitHubHostURL ensures the host URL has https:// scheme and no trailing slashes. +// It is safe to call with URLs that already have the https:// scheme. +func NormalizeGitHubHostURL(rawHostURL string) string { + // Remove all trailing slashes + normalized := strings.TrimRight(rawHostURL, "/") + + // Add https:// scheme if no scheme is present + if !strings.HasPrefix(normalized, "https://") && !strings.HasPrefix(normalized, "http://") { + normalized = "https://" + normalized + } + + return normalized +} + // ExtractDomainFromURL extracts the domain name from a URL string. // Handles various URL formats including full URLs with protocols, URLs with ports, // and plain domain names. diff --git a/pkg/workflow/action_resolver.go b/pkg/workflow/action_resolver.go index aa0df9bd979..37b6f3114d5 100644 --- a/pkg/workflow/action_resolver.go +++ b/pkg/workflow/action_resolver.go @@ -6,6 +6,7 @@ import ( "strings" "time" + "github.com/github/gh-aw/pkg/gitutil" "github.com/github/gh-aw/pkg/logger" ) @@ -51,7 +52,7 @@ func (r *ActionResolver) ResolveSHA(repo, version string) (string, error) { } resolverLog.Printf("Cache miss for %s@%s, querying GitHub API", repo, version) - resolverLog.Printf("This may take a moment as we query GitHub API at /repos/%s/git/ref/tags/%s", extractBaseRepo(repo), version) + resolverLog.Printf("This may take a moment as we query GitHub API at /repos/%s/git/ref/tags/%s", gitutil.ExtractBaseRepo(repo), version) // Resolve using GitHub CLI sha, err := r.resolveFromGitHub(repo, version) @@ -75,7 +76,7 @@ func (r *ActionResolver) ResolveSHA(repo, version string) (string, error) { // resolveFromGitHub uses gh CLI to resolve the SHA for an action@version func (r *ActionResolver) resolveFromGitHub(repo, version string) (string, error) { // Extract base repository (for actions like "github/codeql-action/upload-sarif") - baseRepo := extractBaseRepo(repo) + baseRepo := gitutil.ExtractBaseRepo(repo) resolverLog.Printf("Extracted base repository: %s from %s", baseRepo, repo) // Use gh api to get the git ref for the tag @@ -105,15 +106,3 @@ func (r *ActionResolver) resolveFromGitHub(repo, version string) (string, error) return sha, nil } - -// extractBaseRepo extracts the base repository from a repo path -// For "actions/checkout" -> "actions/checkout" -// For "github/codeql-action/upload-sarif" -> "github/codeql-action" -func extractBaseRepo(repo string) string { - parts := strings.Split(repo, "/") - if len(parts) >= 2 { - // Take first two parts (owner/repo) - return parts[0] + "/" + parts[1] - } - return repo -} diff --git a/pkg/workflow/action_resolver_test.go b/pkg/workflow/action_resolver_test.go index f78fdc484d8..1b72564ebdb 100644 --- a/pkg/workflow/action_resolver_test.go +++ b/pkg/workflow/action_resolver_test.go @@ -6,6 +6,7 @@ import ( "strings" "testing" + "github.com/github/gh-aw/pkg/gitutil" "github.com/github/gh-aw/pkg/testutil" ) @@ -39,9 +40,9 @@ func TestExtractBaseRepo(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := extractBaseRepo(tt.repo) + result := gitutil.ExtractBaseRepo(tt.repo) if result != tt.expected { - t.Errorf("extractBaseRepo(%q) = %q, want %q", tt.repo, result, tt.expected) + t.Errorf("gitutil.ExtractBaseRepo(%q) = %q, want %q", tt.repo, result, tt.expected) } }) } diff --git a/pkg/workflow/git_helpers.go b/pkg/workflow/git_helpers.go index f6b1bac57c8..9936c7dc36f 100644 --- a/pkg/workflow/git_helpers.go +++ b/pkg/workflow/git_helpers.go @@ -42,6 +42,7 @@ import ( "strings" "github.com/github/gh-aw/pkg/console" + "github.com/github/gh-aw/pkg/gitutil" "github.com/github/gh-aw/pkg/logger" "github.com/github/gh-aw/pkg/tty" ) @@ -53,13 +54,11 @@ var gitHelpersLog = logger.New("workflow:git_helpers") // This function is safe to call from any context and won't cause errors if git is not available. func findGitRoot() string { gitHelpersLog.Print("Attempting to find git root directory") - cmd := exec.Command("git", "rev-parse", "--show-toplevel") - output, err := cmd.Output() + gitRoot, err := gitutil.FindGitRoot() if err != nil { gitHelpersLog.Printf("Could not find git root (not a git repo or git not available): %v", err) return "" } - gitRoot := strings.TrimSpace(string(output)) gitHelpersLog.Printf("Found git root: %s", gitRoot) return gitRoot } From d475bcd400f4d03f8947142dafed70cd1314b4a7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 3 Mar 2026 02:55:59 +0000 Subject: [PATCH 3/4] fix: improve NormalizeGitHubHostURL doc comment and add unit tests Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/stringutil/urls.go | 4 +-- pkg/stringutil/urls_test.go | 51 +++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/pkg/stringutil/urls.go b/pkg/stringutil/urls.go index 34c182a1c88..b2fd023e0f9 100644 --- a/pkg/stringutil/urls.go +++ b/pkg/stringutil/urls.go @@ -5,8 +5,8 @@ import ( "strings" ) -// NormalizeGitHubHostURL ensures the host URL has https:// scheme and no trailing slashes. -// It is safe to call with URLs that already have the https:// scheme. +// NormalizeGitHubHostURL ensures the host URL has a scheme (defaulting to https://) and no trailing slashes. +// It is safe to call with URLs that already have an http:// or https:// scheme. func NormalizeGitHubHostURL(rawHostURL string) string { // Remove all trailing slashes normalized := strings.TrimRight(rawHostURL, "/") diff --git a/pkg/stringutil/urls_test.go b/pkg/stringutil/urls_test.go index 17c3373e12e..5b44df395b6 100644 --- a/pkg/stringutil/urls_test.go +++ b/pkg/stringutil/urls_test.go @@ -133,3 +133,54 @@ func BenchmarkExtractDomainFromURL(b *testing.B) { }) } } + +func TestNormalizeGitHubHostURL(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "adds https scheme to plain hostname", + input: "myorg.ghe.com", + expected: "https://myorg.ghe.com", + }, + { + name: "preserves existing https scheme", + input: "https://github.com", + expected: "https://github.com", + }, + { + name: "preserves existing http scheme", + input: "http://github.example.com", + expected: "http://github.example.com", + }, + { + name: "removes single trailing slash", + input: "https://github.com/", + expected: "https://github.com", + }, + { + name: "removes multiple trailing slashes", + input: "https://github.com///", + expected: "https://github.com", + }, + { + name: "adds https and removes trailing slash from plain hostname", + input: "myorg.ghe.com/", + expected: "https://myorg.ghe.com", + }, + { + name: "public GitHub URL unchanged", + input: "https://github.com", + expected: "https://github.com", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := NormalizeGitHubHostURL(tt.input) + assert.Equal(t, tt.expected, result, "Normalized URL should match expected") + }) + } +} From 2d8c14b4117e74687132226da956232a3fa08e84 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 3 Mar 2026 03:16:35 +0000 Subject: [PATCH 4/4] fix: update integration test to use gitutil.ExtractBaseRepo Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/update_actions_integration_test.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/pkg/cli/update_actions_integration_test.go b/pkg/cli/update_actions_integration_test.go index 4c7567d6975..162143855b1 100644 --- a/pkg/cli/update_actions_integration_test.go +++ b/pkg/cli/update_actions_integration_test.go @@ -4,6 +4,8 @@ package cli import ( "testing" + + "github.com/github/gh-aw/pkg/gitutil" ) func TestExtractBaseRepo_Integration(t *testing.T) { @@ -29,9 +31,9 @@ func TestExtractBaseRepo_Integration(t *testing.T) { for actionPath, expectedBase := range realWorldCases { t.Run(actionPath, func(t *testing.T) { - got := extractBaseRepo(actionPath) + got := gitutil.ExtractBaseRepo(actionPath) if got != expectedBase { - t.Errorf("extractBaseRepo(%q) = %q, want %q", actionPath, got, expectedBase) + t.Errorf("gitutil.ExtractBaseRepo(%q) = %q, want %q", actionPath, got, expectedBase) } }) } @@ -56,11 +58,11 @@ func TestExtractBaseRepoAPICompatibility(t *testing.T) { for _, tc := range testCases { t.Run(tc.actionPath, func(t *testing.T) { - baseRepo := extractBaseRepo(tc.actionPath) + baseRepo := gitutil.ExtractBaseRepo(tc.actionPath) // Verify the base repo matches expected format if baseRepo != tc.wantRepoPath { - t.Errorf("extractBaseRepo(%q) = %q, want %q", tc.actionPath, baseRepo, tc.wantRepoPath) + t.Errorf("gitutil.ExtractBaseRepo(%q) = %q, want %q", tc.actionPath, baseRepo, tc.wantRepoPath) } // Verify it can be used to construct a valid API path