Skip to content

Skip branch-PR validation in validate-pr-target-branch workflow#37988

Merged
AndriySvyryd merged 1 commit into
mainfrom
copilot/simplify-pr-target-branch-validation
Mar 24, 2026
Merged

Skip branch-PR validation in validate-pr-target-branch workflow#37988
AndriySvyryd merged 1 commit into
mainfrom
copilot/simplify-pr-target-branch-validation

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 24, 2026

External contributors can only open PRs from forks, not branches. If a PR originates from a branch in the same repo (head.repo.id === base.repo.id), it must be from an internal contributor — no permission API call or release-branch check needed.

Change

  • Added an early exit immediately after the bot check: if the PR head repo matches the base repo (same-repo branch), skip all validation and return immediately.

This eliminates the getCollaboratorPermissionLevel API call for all internal PRs and simplifies the execution path for the majority of PRs.

…ame repo)

Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dotnet/efcore/sessions/9fa9efe5-51a7-419b-bc7a-79c97dd6778c
@AndriySvyryd AndriySvyryd marked this pull request as ready for review March 24, 2026 18:26
@AndriySvyryd AndriySvyryd requested a review from a team as a code owner March 24, 2026 18:26
Copilot AI review requested due to automatic review settings March 24, 2026 18:26
@AndriySvyryd AndriySvyryd assigned roji and unassigned AndriySvyryd and Copilot Mar 24, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the Validate PR Target Branch workflow to short-circuit validation for PRs originating from branches within the same repository, avoiding unnecessary permission checks for internal PRs while keeping fork-based PR validation intact.

Changes:

  • Added an early return when pr.head.repo.id === pr.base.repo.id (same-repo PR), skipping the collaborator permission API call and subsequent release-branch validation logic.

@AndriySvyryd AndriySvyryd enabled auto-merge (squash) March 24, 2026 18:41
}

// If the PR is from a branch (not a fork), it must be from an internal contributor
if (pr.head.repo && pr.head.repo.id === pr.base.repo.id) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks reasonable overall, but shouldn't we just check directly if the PR author has write access? That seems more direct, and also I don't push my PR branches to EF - I push them to my fork just like any external contributor, and it would be good for those PRs to also get picked up as from an internal contributor.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oops, didn't notice auto-merge...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks reasonable overall, but shouldn't we just check directly if the PR author has write access? That seems more direct, and also I don't push my PR branches to EF - I push them to my fork just like any external contributor, and it would be good for those PRs to also get picked up as from an internal contributor.

We already check the access below. This is just a short-circuit optimization.

@AndriySvyryd AndriySvyryd merged commit 68bb873 into main Mar 24, 2026
15 checks passed
@AndriySvyryd AndriySvyryd deleted the copilot/simplify-pr-target-branch-validation branch March 24, 2026 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants