From b91e1a563db80e21db42e3ee2f45e6775d60daca Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 20 Aug 2026 04:54:19 +0000 Subject: [PATCH 1/3] Initial plan From 7467d2cce5f52973e0cf9d27f10770b187f16314 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 20 Aug 2026 05:06:20 +0000 Subject: [PATCH 2/3] Consolidate repo and semver helpers Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/list_workflows_command.go | 7 +++---- pkg/cli/logs_run_processor.go | 11 ++++------- pkg/cli/logs_run_processor_test.go | 23 +++++++++++++++++++++++ pkg/cli/preconditions.go | 6 +++--- pkg/cli/project_command.go | 7 +++---- pkg/cli/repo.go | 7 +++---- pkg/cli/semver.go | 14 ++------------ pkg/cli/setup_command_test.go | 1 + pkg/cli/setup_repository.go | 7 +++---- pkg/cli/trial_repository.go | 4 ++-- pkg/workflow/awf_config.go | 5 ++--- 11 files changed, 49 insertions(+), 43 deletions(-) diff --git a/pkg/cli/list_workflows_command.go b/pkg/cli/list_workflows_command.go index 69555301180..1158c0f2c19 100644 --- a/pkg/cli/list_workflows_command.go +++ b/pkg/cli/list_workflows_command.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/parser" + "github.com/github/gh-aw/pkg/repoutil" "github.com/github/gh-aw/pkg/stringutil" "github.com/spf13/cobra" ) @@ -251,12 +252,10 @@ func getRemoteWorkflowFiles(ctx context.Context, repoSpec, workflowPath string, } // Parse owner/repo - repoParts := strings.Split(repoPart, "/") - if len(repoParts) != 2 { + owner, repo, err := repoutil.SplitRepoSlug(repoPart) + if err != nil { return nil, fmt.Errorf("invalid repository format: %s (expected owner/repo or owner/repo@ref)", repoSpec) } - owner = repoParts[0] - repo = repoParts[1] if verbose && !jsonOutput { fmt.Fprintf(os.Stderr, "Fetching workflow files from %s/%s@%s (path: %s)\n", owner, repo, ref, workflowPath) diff --git a/pkg/cli/logs_run_processor.go b/pkg/cli/logs_run_processor.go index ad5e5a2625e..9b5c040811e 100644 --- a/pkg/cli/logs_run_processor.go +++ b/pkg/cli/logs_run_processor.go @@ -24,6 +24,7 @@ import ( "github.com/github/gh-aw/pkg/constants" "github.com/github/gh-aw/pkg/fileutil" "github.com/github/gh-aw/pkg/parser" + "github.com/github/gh-aw/pkg/repoutil" "github.com/github/gh-aw/pkg/stringutil" "github.com/sourcegraph/conc/pool" ) @@ -69,13 +70,9 @@ type runArtifactsConcurrentOptions struct { func buildConcurrentDownloadParams(outputDir string, verbose bool, repoOverride string, artifactFilter []string, evalsOnly bool, artifactSets []string) concurrentRunDownloadParams { var dlHost, dlOwner, dlRepo string if repoOverride != "" { - // Accepted formats: "owner/repo" or "HOST/owner/repo". - parts := strings.SplitN(repoOverride, "/", 3) - switch len(parts) { - case 3: // HOST/owner/repo - dlHost, dlOwner, dlRepo = parts[0], parts[1], parts[2] - case 2: // owner/repo - dlOwner, dlRepo = parts[0], parts[1] + ownerRepo, host := repoutil.NormalizeRepoForAPI(repoOverride) + if owner, repo, err := repoutil.SplitRepoSlug(ownerRepo); err == nil { + dlHost, dlOwner, dlRepo = host, owner, repo } } evalsArtifactRequested := isEvalsArtifactRequested(evalsOnly, artifactSets) diff --git a/pkg/cli/logs_run_processor_test.go b/pkg/cli/logs_run_processor_test.go index b0120030354..4f2e4ee1919 100644 --- a/pkg/cli/logs_run_processor_test.go +++ b/pkg/cli/logs_run_processor_test.go @@ -14,6 +14,29 @@ import ( "github.com/stretchr/testify/require" ) +func TestBuildConcurrentDownloadParams_RepoOverride(t *testing.T) { + tests := []struct { + name string + repoOverride string + wantHost string + wantOwner string + wantRepo string + }{ + {name: "owner and repo", repoOverride: "owner/repo", wantOwner: "owner", wantRepo: "repo"}, + {name: "host owner and repo", repoOverride: "ghe.example/owner/repo", wantHost: "ghe.example", wantOwner: "owner", wantRepo: "repo"}, + {name: "empty component", repoOverride: "owner/"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + params := buildConcurrentDownloadParams("", false, tt.repoOverride, nil, false, nil) + assert.Equal(t, tt.wantHost, params.dlHost) + assert.Equal(t, tt.wantOwner, params.dlOwner) + assert.Equal(t, tt.wantRepo, params.dlRepo) + }) + } +} + func TestRunHasEvals(t *testing.T) { tests := []struct { name string diff --git a/pkg/cli/preconditions.go b/pkg/cli/preconditions.go index fd757f9b32e..bd07dde9dfe 100644 --- a/pkg/cli/preconditions.go +++ b/pkg/cli/preconditions.go @@ -9,6 +9,7 @@ import ( "github.com/github/gh-aw/pkg/console" "github.com/github/gh-aw/pkg/logger" + "github.com/github/gh-aw/pkg/repoutil" "github.com/github/gh-aw/pkg/workflow" ) @@ -164,11 +165,10 @@ func parseJSON(data []byte, v any) error { func checkUserPermissionsShared(repoSlug string, verbose bool) (bool, error) { preconditionsLog.Print("Checking user permissions") - parts := strings.Split(repoSlug, "/") - if len(parts) != 2 { + owner, repo, err := repoutil.SplitRepoSlug(repoSlug) + if err != nil { return false, fmt.Errorf("invalid repository format: %s", repoSlug) } - owner, repo := parts[0], parts[1] hasAccess, err := checkRepositoryAccess(owner, repo) if err != nil { diff --git a/pkg/cli/project_command.go b/pkg/cli/project_command.go index 6a927414cf4..943bdebbf37 100644 --- a/pkg/cli/project_command.go +++ b/pkg/cli/project_command.go @@ -13,6 +13,7 @@ import ( "github.com/github/gh-aw/pkg/console" "github.com/github/gh-aw/pkg/errorutil" "github.com/github/gh-aw/pkg/logger" + "github.com/github/gh-aw/pkg/repoutil" "github.com/github/gh-aw/pkg/workflow" "github.com/spf13/cobra" ) @@ -419,12 +420,10 @@ func linkProjectToRepo(ctx context.Context, projectId, repoSlug string, verbose console.LogVerbose(verbose, "Linking project to repository: "+repoSlug) // Parse repo slug - parts := strings.Split(repoSlug, "/") - if len(parts) != 2 { + repoOwner, repoName, err := repoutil.SplitRepoSlug(repoSlug) + if err != nil { return fmt.Errorf("repository slug '%s' is not in owner/repo format. Expected '/'. Example: github/gh-aw", repoSlug) } - repoOwner := parts[0] - repoName := parts[1] // Get repository ID repoIdQuery := `query($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { id } }` diff --git a/pkg/cli/repo.go b/pkg/cli/repo.go index 99655f43ac3..ea25860be90 100644 --- a/pkg/cli/repo.go +++ b/pkg/cli/repo.go @@ -6,6 +6,7 @@ import ( "strings" "github.com/github/gh-aw/pkg/logger" + "github.com/github/gh-aw/pkg/repoutil" "github.com/github/gh-aw/pkg/syncutil" "github.com/github/gh-aw/pkg/workflow" ) @@ -27,8 +28,7 @@ func getCurrentRepoSlugUncached() (string, error) { repoSlug := strings.TrimSpace(string(output)) if repoSlug != "" { // Validate format (should be owner/repo) - parts := strings.Split(repoSlug, "/") - if len(parts) == 2 && parts[0] != "" && parts[1] != "" { + if _, _, err := repoutil.SplitRepoSlug(repoSlug); err == nil { repoLog.Printf("Successfully got repository slug via gh CLI: %s", repoSlug) return repoSlug, nil } @@ -55,8 +55,7 @@ func getCurrentRepoSlugUncached() (string, error) { } // Validate format (should be owner/repo) - parts := strings.Split(repoPath, "/") - if len(parts) != 2 || parts[0] == "" || parts[1] == "" { + if _, _, err := repoutil.SplitRepoSlug(repoPath); err != nil { repoLog.Printf("Invalid repository format: %s", repoPath) return "", fmt.Errorf("invalid repository format: %s. Expected format: owner/repo. Example: github/gh-aw", repoPath) } diff --git a/pkg/cli/semver.go b/pkg/cli/semver.go index 29dc145fb97..a8e09a64c67 100644 --- a/pkg/cli/semver.go +++ b/pkg/cli/semver.go @@ -1,11 +1,6 @@ package cli -import ( - "github.com/github/gh-aw/pkg/logger" - "github.com/github/gh-aw/pkg/semverutil" -) - -var semverLog = logger.New("cli:semver") +import "github.com/github/gh-aw/pkg/semverutil" // isSemanticVersionTag checks if a ref string looks like a semantic version tag // Uses golang.org/x/mod/semver for proper semantic version validation @@ -16,10 +11,5 @@ func isSemanticVersionTag(ref string) bool { // parseVersion parses a semantic version string and returns a *semverutil.SemanticVersion. // Uses golang.org/x/mod/semver for proper semantic version parsing. func parseVersion(v string) *semverutil.SemanticVersion { - semverLog.Printf("Parsing semantic version: %s", v) - parsed := semverutil.ParseVersion(v) - if parsed == nil { - semverLog.Printf("Invalid semantic version: %s", v) - } - return parsed + return semverutil.ParseVersion(v) } diff --git a/pkg/cli/setup_command_test.go b/pkg/cli/setup_command_test.go index 1566da87817..2f07169f1e5 100644 --- a/pkg/cli/setup_command_test.go +++ b/pkg/cli/setup_command_test.go @@ -222,6 +222,7 @@ func TestRunSetupRepositoryCheck_AcceptsCaseInsensitiveSlugMatch(t *testing.T) { func TestValidateSetupRepositoryCheckOptions_RejectsEmptyRepoComponents(t *testing.T) { tests := []SetupRepositoryCheckOptions{ + {Repo: "/"}, {Repo: "/repo"}, {Repo: "owner/"}, } diff --git a/pkg/cli/setup_repository.go b/pkg/cli/setup_repository.go index 01bf149785b..4cc6eefd54a 100644 --- a/pkg/cli/setup_repository.go +++ b/pkg/cli/setup_repository.go @@ -14,6 +14,7 @@ import ( "github.com/github/gh-aw/pkg/errorutil" "github.com/github/gh-aw/pkg/gitutil" "github.com/github/gh-aw/pkg/logger" + "github.com/github/gh-aw/pkg/repoutil" "github.com/github/gh-aw/pkg/workflow" ) @@ -352,10 +353,8 @@ func validateSetupRepositoryCheckOptions(opts SetupRepositoryCheckOptions) error } func isValidOwnerRepoSlug(repo string) bool { - parts := strings.Split(repo, "/") - return len(parts) == 2 && - strings.TrimSpace(parts[0]) != "" && - strings.TrimSpace(parts[1]) != "" + owner, name, err := repoutil.SplitRepoSlug(repo) + return err == nil && strings.TrimSpace(owner) != "" && strings.TrimSpace(name) != "" } func runSetupRepositoryCheckWithRuntime(opts SetupRepositoryCheckOptions, runtime setupRepositoryRuntime) error { diff --git a/pkg/cli/trial_repository.go b/pkg/cli/trial_repository.go index 2f4b4d85e2a..bda0142df45 100644 --- a/pkg/cli/trial_repository.go +++ b/pkg/cli/trial_repository.go @@ -15,6 +15,7 @@ import ( "github.com/github/gh-aw/pkg/fileutil" "github.com/github/gh-aw/pkg/gitutil" "github.com/github/gh-aw/pkg/logger" + "github.com/github/gh-aw/pkg/repoutil" "github.com/github/gh-aw/pkg/workflow" ) @@ -75,8 +76,7 @@ func trialRepositoryActionsSettingsURL(repoSlug string) string { func ensureTrialRepository(repoSlug string, cloneRepoSlug string, forceDeleteHostRepo bool, dryRun bool, verbose bool) error { trialRepoLog.Printf("Ensuring trial repository: %s (cloneRepo=%s, forceDelete=%v, dryRun=%v)", repoSlug, cloneRepoSlug, forceDeleteHostRepo, dryRun) - parts := strings.Split(repoSlug, "/") - if len(parts) != 2 || parts[0] == "" || parts[1] == "" { + if _, _, err := repoutil.SplitRepoSlug(repoSlug); err != nil { return fmt.Errorf("invalid repository slug format: %s. Expected format: owner/repo. Example: github/gh-aw", repoSlug) } diff --git a/pkg/workflow/awf_config.go b/pkg/workflow/awf_config.go index 8c84bf160a9..dfcb529da42 100644 --- a/pkg/workflow/awf_config.go +++ b/pkg/workflow/awf_config.go @@ -78,6 +78,7 @@ import ( "github.com/github/gh-aw/pkg/constants" "github.com/github/gh-aw/pkg/jsonutil" "github.com/github/gh-aw/pkg/logger" + "github.com/github/gh-aw/pkg/semverutil" "github.com/github/gh-aw/pkg/setutil" "github.com/github/gh-aw/pkg/syncutil" "github.com/github/gh-aw/pkg/workflow/compilerenv" @@ -455,9 +456,7 @@ func buildAWFConfigSchemaURL(firewallConfig *FirewallConfig) string { return "https://github.com/github/gh-aw-firewall/releases/latest/download/awf-config.schema.json" } // Ensure version has the 'v' prefix required by GitHub release tag URLs. - if !strings.HasPrefix(version, "v") { - version = "v" + version - } + version = semverutil.EnsureVPrefix(version) return fmt.Sprintf("https://github.com/github/gh-aw-firewall/releases/download/%s/awf-config.schema.json", version) } From 007159b20ee6128f9f8034cde22e0a9d1516a7fe Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 20 Aug 2026 05:08:06 +0000 Subject: [PATCH 3/3] Cover invalid host-qualified repo override Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/logs_run_processor_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/cli/logs_run_processor_test.go b/pkg/cli/logs_run_processor_test.go index 4f2e4ee1919..757c9ed8ea9 100644 --- a/pkg/cli/logs_run_processor_test.go +++ b/pkg/cli/logs_run_processor_test.go @@ -25,6 +25,7 @@ func TestBuildConcurrentDownloadParams_RepoOverride(t *testing.T) { {name: "owner and repo", repoOverride: "owner/repo", wantOwner: "owner", wantRepo: "repo"}, {name: "host owner and repo", repoOverride: "ghe.example/owner/repo", wantHost: "ghe.example", wantOwner: "owner", wantRepo: "repo"}, {name: "empty component", repoOverride: "owner/"}, + {name: "empty host-qualified component", repoOverride: "ghe.example/owner/"}, } for _, tt := range tests {