Skip to content

Add WithHead option to prcreator for App-auth PR creation #4974

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jmguzik wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add WithHead option to prcreator for App-auth PR creation #4974

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d2b6d11
Add WithHead option to prcreator for App-auth PR creation
jmguzik Feb 27, 2026
df6cd87
Add GitHub App authentication support to prcreator
hbhushan3 Mar 3, 2026
cefc785
Merge pull request #3 from hbhushan3/update-prcreation
jmguzik Mar 4, 2026
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
14 changes: 11 additions & 3 deletions cmd/prcreator/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ type options struct {
organization string
repo string
branch string
head string
}

func gatherOptions() (*options, error) {
Expand All @@ -32,6 +33,7 @@ func gatherOptions() (*options, error) {
flag.StringVar(&opts.organization, "organization", "openshift", "The GitHub organization in which the PR should be created")
flag.StringVar(&opts.repo, "repo", "release", "The name of the repo in which the PR should be created")
flag.StringVar(&opts.branch, "branch", "main", "The branch for which the PR should be created")
flag.StringVar(&opts.head, "head", "", "Pre-pushed head ref (e.g. user:branch). When set, skips fork/commit/push and only creates/updates the PR")
flag.Parse()

var errs []error
Expand Down Expand Up @@ -61,14 +63,20 @@ func main() {
logrus.WithError(err).Fatal("failed to gather options")
}

var prOpts []prcreation.PrOption
prOpts = append(prOpts, prcreation.PrBody(opts.prMessage))
prOpts = append(prOpts, prcreation.PrAssignee(opts.prAssignee))
prOpts = append(prOpts, prcreation.GitCommitMessage(opts.gitCommitMessage))
if opts.head != "" {
prOpts = append(prOpts, prcreation.WithHead(opts.head))
}

if err := opts.PRCreationOptions.UpsertPR(".",
opts.organization,
opts.repo,
opts.branch,
opts.prTitle,
prcreation.PrBody(opts.prMessage),
prcreation.PrAssignee(opts.prAssignee),
prcreation.GitCommitMessage(opts.gitCommitMessage),
prOpts...,
); err != nil {
logrus.WithError(err).Fatal("failed to upsert PR")
}
Expand Down
17 changes: 17 additions & 0 deletions pkg/github/prcreation/orgaware.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package prcreation

import "sigs.k8s.io/prow/pkg/github"

// OrgAwareClient wraps a github.Client so that FindIssues routes through
// FindIssuesWithOrg. Prow's App auth round-tripper requires the org in
// the request context to resolve the installation token, but
// bumper.UpdatePullRequestWithLabels internally calls FindIssues which
// passes an empty org.
type OrgAwareClient struct {
github.Client
Org string
}

func (c *OrgAwareClient) FindIssues(query, sort string, asc bool) ([]github.Issue, error) {
return c.Client.FindIssuesWithOrg(c.Org, query, sort, asc)
}
77 changes: 53 additions & 24 deletions pkg/github/prcreation/prcreation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,10 @@ func (o *PRCreationOptions) Finalize() error {
if err := o.GitHubOptions.Validate(false); err != nil {
return err
}
if err := secret.Add(o.TokenPath); err != nil {
return fmt.Errorf("failed to start secretAgent: %w", err)
if o.TokenPath != "" {
if err := secret.Add(o.TokenPath); err != nil {
return fmt.Errorf("failed to start secretAgent: %w", err)
}
}
var err error
o.GithubClient, err = o.GitHubClient(false)
Expand All @@ -51,6 +53,7 @@ type PrOptions struct {
prAssignee string
gitCommitMessage string
skipPRCreation bool
head string
}

// PrOption is the type for Optional Parameters
Expand Down Expand Up @@ -99,6 +102,14 @@ func SkipPRCreation() PrOption {
}
}

// WithHead skips all git operations (fork, commit, push) and jumps
// straight to PR creation using the provided head ref (e.g. "user:branch").
func WithHead(head string) PrOption {
return func(args *PrOptions) {
args.head = head
}
}

// UpsertPR upserts a PR. The PRTitle must be alphanumeric except for spaces, as it will be used as the
// branchname on the bots fork.
func (o *PRCreationOptions) UpsertPR(localSourceDir, org, repo, branch, prTitle string, setters ...PrOption) error {
Expand All @@ -109,6 +120,11 @@ func (o *PRCreationOptions) UpsertPR(localSourceDir, org, repo, branch, prTitle
if prArgs.matchTitle == "" {
prArgs.matchTitle = prTitle
}

if prArgs.head != "" {
return o.upsertPR(org, repo, branch, prTitle, prArgs)
}
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Feb 27, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Validate execution mode inputs before branching.

This branch should fail fast for malformed --head and for non-head mode without a token path; otherwise failures happen later during API/git operations and are harder to diagnose.

Suggested fix 
 	if prArgs.head != "" {
+		parts := strings.SplitN(prArgs.head, ":", 2)
+		if len(parts) != 2 || parts[0] == "" || parts[1] == "" {
+			return fmt.Errorf("invalid --head value %q, expected <owner>:<branch>", prArgs.head)
+		}
 		return o.upsertPR(org, repo, branch, prTitle, prArgs)
 	}
+
+	if o.TokenPath == "" {
+		return fmt.Errorf("token path is required when --head is not set")
+	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if prArgs.head != "" {
return o.upsertPR(org, repo, branch, prTitle, prArgs)
}
if prArgs.head != "" {
parts := strings.SplitN(prArgs.head, ":", 2)
if len(parts) != 2 || parts[0] == "" || parts[1] == "" {
return fmt.Errorf("invalid --head value %q, expected <owner>:<branch>", prArgs.head)
}
return o.upsertPR(org, repo, branch, prTitle, prArgs)
}
if o.TokenPath == "" {
return fmt.Errorf("token path is required when --head is not set")
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@pkg/github/prcreation/prcreation.go` around lines 124 - 126, Before calling
upsertPR, add fast-fail validation of inputs: if prArgs.head != "" validate the
head value format (e.g., non-empty and matches expected "user:branch" or
"owner/branch" pattern) and return a clear error if malformed; otherwise (when
prArgs.head == "") ensure a token path is provided (e.g., prArgs.tokenPath is
non-empty) and return an error if missing. Place these checks immediately before
the existing conditional that calls o.upsertPR(org, repo, branch, prTitle,
prArgs) so invalid inputs are rejected early.


if err := os.Chdir(localSourceDir); err != nil {
return fmt.Errorf("failed to chdir into %s: %w", localSourceDir, err)
}
Comment on lines +167 to 170
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 4, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Restore the original working directory after os.Chdir.

Both paths mutate process-global cwd and never restore it. In long-lived callers, later file/git ops can run in the wrong directory.

Proposed fix 
 func (o *PRCreationOptions) upsertWithPAT(localSourceDir, org, repo, branch, prTitle string, prArgs *PrOptions) error {
+	cwd, err := os.Getwd()
+	if err != nil {
+		return fmt.Errorf("failed to get current working directory: %w", err)
+	}
 	if err := os.Chdir(localSourceDir); err != nil {
 		return fmt.Errorf("failed to chdir into %s: %w", localSourceDir, err)
 	}
+	defer func() {
+		_ = os.Chdir(cwd)
+	}()
-	changed, err := bumper.HasChanges()
+	changed, err := bumper.HasChanges()
 	if err != nil {
 		return fmt.Errorf("failed to check for changes: %w", err)
 	}
 func (o *PRCreationOptions) upsertWithAppAuth(localSourceDir, org, repo, branch, prTitle string, prArgs *PrOptions) error {
+	cwd, err := os.Getwd()
+	if err != nil {
+		return fmt.Errorf("failed to get current working directory: %w", err)
+	}
 	if err := os.Chdir(localSourceDir); err != nil {
 		return fmt.Errorf("failed to chdir into %s: %w", localSourceDir, err)
 	}
+	defer func() {
+		_ = os.Chdir(cwd)
+	}()
-	changed, err := bumper.HasChanges()
+	changed, err := bumper.HasChanges()
 	if err != nil {
 		return fmt.Errorf("failed to check for changes: %w", err)
 	}

Also applies to: 244-246

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@pkg/github/prcreation/prcreation.go` around lines 167 - 170, The functions
that call os.Chdir (notably upsertWithPAT) change the global working directory
and never restore it; capture the original working directory with os.Getwd()
before calling os.Chdir, then defer restoring it (e.g., defer func() { _ =
os.Chdir(origWd) }()) so the process cwd is always reset even on early returns;
apply the same pattern to the other function(s) in this file that perform
os.Chdir so all paths restore the original directory and wrap or return an error
if Getwd fails.

Expand Down Expand Up @@ -192,35 +208,48 @@ func (o *PRCreationOptions) UpsertPR(localSourceDir, org, repo, branch, prTitle
return nil
}

prArgs.head = username + ":" + sourceBranchName
return o.upsertPR(org, repo, branch, prTitle, prArgs)
}

// upsertPR creates or updates a PR for a pre-pushed head ref.
func (o *PRCreationOptions) upsertPR(org, repo, branch, prTitle string, prArgs *PrOptions) error {
headBranch := prArgs.head
if i := strings.Index(headBranch, ":"); i != -1 {
headBranch = headBranch[i+1:]
}

labelsToAdd := prArgs.additionalLabels
if o.SelfApprove {
l.Infof("Self-aproving PR by adding the %q and %q labels", labels.Approved, labels.LGTM)
logrus.Infof("Self-aproving PR by adding the %q and %q labels", labels.Approved, labels.LGTM)
labelsToAdd = append(labelsToAdd, labels.Approved, labels.LGTM)
}
Comment on lines 322 to 326
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 4, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Copy additionalLabels before appending self-approval labels.

labelsToAdd := prArgs.additionalLabels aliases the caller slice; append may mutate caller-owned data when capacity allows.

Proposed fix 
-	labelsToAdd := prArgs.additionalLabels
+	labelsToAdd := append([]string(nil), prArgs.additionalLabels...)
 	if o.SelfApprove {
 		logrus.Infof("Self-approving PR by adding the %q and %q labels", labels.Approved, labels.LGTM)
 		labelsToAdd = append(labelsToAdd, labels.Approved, labels.LGTM)
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
labelsToAdd := prArgs.additionalLabels
if o.SelfApprove {
l.Infof("Self-aproving PR by adding the %q and %q labels", labels.Approved, labels.LGTM)
logrus.Infof("Self-approving PR by adding the %q and %q labels", labels.Approved, labels.LGTM)
labelsToAdd = append(labelsToAdd, labels.Approved, labels.LGTM)
}
labelsToAdd := append([]string(nil), prArgs.additionalLabels...)
if o.SelfApprove {
logrus.Infof("Self-approving PR by adding the %q and %q labels", labels.Approved, labels.LGTM)
labelsToAdd = append(labelsToAdd, labels.Approved, labels.LGTM)
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@pkg/github/prcreation/prcreation.go` around lines 322 - 326,
prArgs.additionalLabels is being aliased by labelsToAdd so appending
self-approval labels can mutate the caller's slice; change the assignment in the
PR creation flow to make a copy (e.g. create a new slice and copy contents of
prArgs.additionalLabels into labelsToAdd) before the SelfApprove branch, then
append labels.Approved and labels.LGTM when o.SelfApprove is true so callers'
slices are not modified.


assignees := strings.Split(prArgs.prAssignee, ",")
for i, assignee := range assignees {
assignees[i] = "@" + assignee
prBody := prArgs.prBody + formatAssigneeCC(prArgs.prAssignee)
prClient := github.Client(&OrgAwareClient{Client: o.GithubClient, Org: org})

return bumper.UpdatePullRequestWithLabels(
prClient, org, repo, prTitle, prBody,
prArgs.head, branch, headBranch,
true, labelsToAdd, false,
)
}

func formatAssigneeCC(assigneeCSV string) string {
if assigneeCSV == "" {
return ""
}
assigneeList := strings.Join(assignees, ", ")

if err := bumper.UpdatePullRequestWithLabels(
o.GithubClient,
org,
repo,
prTitle,
prArgs.prBody+"\n/cc "+assigneeList,
username+":"+sourceBranchName,
branch,
sourceBranchName,
true,
labelsToAdd,
false,
); err != nil {
return fmt.Errorf("failed to upsert PR: %w", err)
var mentions []string
for _, a := range strings.Split(assigneeCSV, ",") {
a = strings.TrimSpace(a)
if a != "" {
mentions = append(mentions, "@"+a)
}
}

return nil
if len(mentions) == 0 {
return ""
}
return "\n/cc " + strings.Join(mentions, ", ")
}

func waitForRepo(owner, name string, ghc github.Client) error {
Expand Down