Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 37 additions & 28 deletions pkg/cli/pr_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,28 +96,6 @@ func parsePRURL(prURL string) (owner, repo string, prNumber int, err error) {
return parser.ParsePRURL(prURL)
}

// getCurrentRepo gets the current repository information using gh CLI
func getCurrentRepo() (owner, repo string, err error) {
cmd := exec.Command("gh", "repo", "view", "--json", "owner,name")
output, err := cmd.Output()
if err != nil {
return "", "", fmt.Errorf("failed to get current repository info: %w", err)
}

var repoInfo struct {
Owner struct {
Login string `json:"login"`
} `json:"owner"`
Name string `json:"name"`
}

if err := json.Unmarshal(output, &repoInfo); err != nil {
return "", "", fmt.Errorf("failed to parse repository info: %w", err)
}

return repoInfo.Owner.Login, repoInfo.Name, nil
}

// checkRepositoryAccess checks if the current user has write access to the target repository
func checkRepositoryAccess(owner, repo string) (bool, error) {
// Get current user
Expand Down Expand Up @@ -593,10 +571,14 @@ func transferPR(prURL, targetRepo string, verbose bool) error {
targetOwner, targetRepoName = parts[0], parts[1]
} else {
// Use current repository as target
targetOwner, targetRepoName, err = getCurrentRepo()
slug, err := GetCurrentRepoSlug()
if err != nil {
return fmt.Errorf("failed to determine target repository: %w", err)
}
targetOwner, targetRepoName, err = SplitRepoSlug(slug)
if err != nil {
return fmt.Errorf("failed to parse target repository: %w", err)
}
}

if verbose {
Expand All @@ -615,12 +597,39 @@ func transferPR(prURL, targetRepo string, verbose bool) error {
if targetRepo != "" {
// Check if we're already in the target repository
if isGitRepo() {
currentOwner, currentRepoName, err := getCurrentRepo()
if err == nil && currentOwner == targetOwner && currentRepoName == targetRepoName {
// We're already in the target repo
workingDir = "."
slug, err := GetCurrentRepoSlug()
if err == nil {
currentOwner, currentRepoName, err := SplitRepoSlug(slug)
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variable shadowing issue: the err variable from line 600 is shadowed by the := assignment on line 602. This means if SplitRepoSlug returns an error, the outer error check on line 603 will evaluate the wrong error (from GetCurrentRepoSlug). Use var currentOwner, currentRepoName string before line 602 and then currentOwner, currentRepoName, err = SplitRepoSlug(slug) with = instead of :=.

Suggested change
currentOwner, currentRepoName, err := SplitRepoSlug(slug)
var currentOwner, currentRepoName string
currentOwner, currentRepoName, err = SplitRepoSlug(slug)

Copilot uses AI. Check for mistakes.
if err == nil && currentOwner == targetOwner && currentRepoName == targetRepoName {
// We're already in the target repo
workingDir = "."
} else {
// We need to clone the target repository
if verbose {
fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Cloning target repository %s/%s...", targetOwner, targetRepoName)))
}
tempDir, err := os.MkdirTemp("", "gh-aw-pr-transfer-repo-")
if err != nil {
return fmt.Errorf("failed to create temp directory for repo: %w", err)
}

cloneCmd := exec.Command("gh", "repo", "clone", fmt.Sprintf("%s/%s", targetOwner, targetRepoName), tempDir)
if err := cloneCmd.Run(); err != nil {
os.RemoveAll(tempDir)
return fmt.Errorf("failed to clone target repository: %w", err)
}

workingDir = tempDir
needsCleanup = true

// Change to the cloned repository directory
if err := os.Chdir(tempDir); err != nil {
os.RemoveAll(tempDir)
return fmt.Errorf("failed to change to cloned repository directory: %w", err)
}
}
} else {
// We need to clone the target repository
// Error getting current repo, clone anyway
if verbose {
fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Cloning target repository %s/%s...", targetOwner, targetRepoName)))
}
Expand Down
9 changes: 9 additions & 0 deletions pkg/cli/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,3 +104,12 @@ func GetCurrentRepoSlug() (string, error) {
repoLog.Printf("Using cached repository slug: %s", currentRepoSlugResult)
return currentRepoSlugResult, nil
}

// SplitRepoSlug splits "owner/repo" into owner and repo
func SplitRepoSlug(slug string) (owner, repo string, err error) {
parts := strings.Split(slug, "/")
if len(parts) != 2 {
return "", "", fmt.Errorf("invalid repo format: %s", slug)
}
return parts[0], parts[1], nil
}
Comment on lines +109 to +115
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SplitRepoSlug function does not validate that owner and repo parts are non-empty strings. The existing getCurrentRepoSlugUncached function (lines 42-43 and 84) checks both len(parts) == 2 AND parts[0] != "" && parts[1] != "". This inconsistency means SplitRepoSlug would accept invalid slugs like "/repo" or "owner/" that the rest of the codebase rejects. Add validation: if len(parts) != 2 || parts[0] == "" || parts[1] == "" { ... }

Copilot uses AI. Check for mistakes.
4 changes: 2 additions & 2 deletions pkg/cli/spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func parseRepoSpec(repoSpec string) (*RepoSpec, error) {
repo = fmt.Sprintf("%s/%s", pathParts[0], pathParts[1])
} else if repo == "." {
// Handle current directory as repo (local workflow)
currentRepo, err := getCurrentRepositoryInfo()
currentRepo, err := GetCurrentRepoSlug()
if err != nil {
return nil, fmt.Errorf("failed to get current repository info: %w", err)
}
Expand Down Expand Up @@ -269,7 +269,7 @@ func parseLocalWorkflowSpec(spec string) (*WorkflowSpec, error) {
}

// Get current repository info
repoInfo, err := getCurrentRepositoryInfo()
repoInfo, err := GetCurrentRepoSlug()
if err != nil {
return nil, fmt.Errorf("failed to get current repository info for local workflow: %w", err)
}
Expand Down
8 changes: 1 addition & 7 deletions pkg/cli/trial_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ func RunWorkflowTrials(workflowSpecs []string, logicalRepoSpec string, cloneRepo
} else {
// Fall back to current repository for logical-repo mode
var err error
logicalRepoSlug, err = getCurrentRepositoryInfo()
logicalRepoSlug, err = GetCurrentRepoSlug()
if err != nil {
return fmt.Errorf("failed to determine simulated host repository: %w", err)
}
Expand Down Expand Up @@ -456,12 +456,6 @@ func RunWorkflowTrials(workflowSpecs []string, logicalRepoSpec string, cloneRepo

}

// getCurrentRepositoryInfo determines the current repository from the gh CLI (cached)
// This is a wrapper around GetCurrentRepoSlug for backward compatibility
func getCurrentRepositoryInfo() (string, error) {
return GetCurrentRepoSlug()
}

// getCurrentGitHubUsername gets the current GitHub username from gh CLI
func getCurrentGitHubUsername() (string, error) {
cmd := exec.Command("gh", "api", "user", "--jq", ".login")
Expand Down
Loading